Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When clicking an inactive feature in direct_select, select it #978

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

trygveaa
Copy link

With this, you don't have to click twice to select another feature when
you are in direct_select.

The mode will still be changed to simple_select. So if you want to
activate direct_select for another feature, you have to click it twice,
but that's still one less click than before.

@usb248
Copy link

usb248 commented Mar 24, 2020

Good ! But cursor pointer (hover) on inactive feature is missing @trygveaa

@trygveaa trygveaa force-pushed the direct-select-select-inactive-feature branch from 3aa2ab3 to 0d091e1 Compare March 25, 2020 17:27
@trygveaa
Copy link
Author

@usb248: Ah, right. I fixed it now.

@trygveaa trygveaa force-pushed the direct-select-select-inactive-feature branch from 0d091e1 to 7acd0b7 Compare March 25, 2020 21:16
@karimnaaji karimnaaji closed this Jun 16, 2020
@karimnaaji karimnaaji reopened this Jun 16, 2020
@arindam1993 arindam1993 changed the base branch from master to main June 17, 2020 01:23
Copy link
Contributor

@adrianababakanian adrianababakanian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening @trygveaa -- this is working well. I'd advise a small update to the test case before merging.

Comment on lines 283 to 292
afterNextRender(() => {
click(map, makeMouseEvent(clickAt[0], clickAt[1]));
afterNextRender(() => {
t.equal(Draw.getSelectedIds().indexOf(polygonId) !== -1, true, 'polygon is now selected');
cleanUp(() => st.end());
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since a selection change is occurring, it is worth checking that the correct event is fired.

Suggested change
afterNextRender(() => {
click(map, makeMouseEvent(clickAt[0], clickAt[1]));
afterNextRender(() => {
t.equal(Draw.getSelectedIds().indexOf(polygonId) !== -1, true, 'polygon is now selected');
cleanUp(() => st.end());
});
});
afterNextRender(() => {
click(map, makeMouseEvent(clickAt[0], clickAt[1]));
map.fire.resetHistory();
afterNextRender(() => {
const args = getFireArgs().filter(arg => arg[0] === 'draw.selectionchange');
t.equal(args.length, 1, 'should have one and only one selectionchange event');
t.equal(Draw.getSelectedIds().indexOf(polygonId) !== -1, true, 'polygon is now selected');
cleanUp(() => st.end());
});
});

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, applied.

With this, you don't have to click twice to select another feature when
you are in direct_select.

The mode will still be changed to simple_select. So if you want to
activate direct_select for another feature, you have to click it twice,
but that's still one less click than before.
@trygveaa trygveaa force-pushed the direct-select-select-inactive-feature branch from 00bc6a4 to aec298f Compare July 8, 2020 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants