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

Don't enter delete or edit modes if no layers are available #136

Closed
wants to merge 3 commits into from

Conversation

snkashis
Copy link
Member

@snkashis snkashis commented May 3, 2013

I thought it was a bit odd that you could enter the delete and edit modes and see the tooltip+save/cancel buttons, even if there was nothing available to use them on. This prevents that.

@jacobtoye
Copy link
Member

I'm inclined to think that it should enter edit mode, if only to show the user that something happened. It would be quite strange if the user clicked a button and nothing happened. I don't think it is Leaflet.draw's place to show an error either.

Thoughts?

@Starefossen
Copy link
Contributor

What about disabling the buttons (greyed out) until there is something to edit/delete?!

@snkashis
Copy link
Member Author

I think that makes a lot of sense, and better than my original pull.

On Thu, May 30, 2013 at 12:21 PM, Hans Kristian Flaatten <
notifications@github.com> wrote:

What about disabling the buttons (greyed out) until there is something to
edit/delete?!


Reply to this email directly or view it on GitHubhttps://github.com//pull/136#issuecomment-18691371
.

@jacobtoye
Copy link
Member

Yeah that's a good idea @Starefossen :) Should be pretty simple to implement. Have a css class for disabled state and set it when the handler is enabled.

@snkashis
Copy link
Member Author

Okay, just added that in. Watches created and deleted events.

@cazacugmihai
Copy link
Contributor

+1 for this fix. Why is still not merged?

@markgibbons25
Copy link

+1. Perhaps also a tooltip when you hover saying "No items to edit".

@jacobtoye
Copy link
Member

Thanks for the work @snkashis. I've implemented this a bit differently so closing this pull.

@jacobtoye jacobtoye closed this Sep 30, 2013
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.

5 participants