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

Allow user to remove layer from (DOM) map / layer control #253

Closed
prushforth opened this issue Jan 22, 2021 · 5 comments · Fixed by #306
Closed

Allow user to remove layer from (DOM) map / layer control #253

prushforth opened this issue Jan 22, 2021 · 5 comments · Fixed by #306

Comments

@prushforth
Copy link
Member

This is just an idea for discussion.

It might be useful to allow the user to not just remove a layer from the map (checked), but to remove the layer from the DOM altogether, e.g. click X to delete it from the layer control. It might be confusing though too, so maybe not. Please add your thoughts below.

Does such an idea affect requirements, or specifications?

@Malvoz
Copy link
Member

Malvoz commented Jan 22, 2021

I agree with this, especially if users can add their own layers.

Here's a quick mockup of a potential UI (using display: flex and align-items: center on the summary element, with a new sibling to the label element):

delete-layer-button-mockup

× should ideally be a button (whether <button> or <a role="button">) with ARIA properties such as aria-label="Delete layer" and maybe aria-controls="{ID-of-parent-fieldset-el}".

Does such an idea affect requirements, or specifications?

This is an implementation detail that probably should not be subject to the UCR report. I do think this is worthy of mentioning in the spec, along with other descriptions of the UI of the individual controls.

@Malvoz
Copy link
Member

Malvoz commented Jan 22, 2021

Should it be possible to restore a deleted layer? Maybe I deleted the wrong one and want it back, without refreshing the page.

Would it make sense that the reload control also handles the restoration of either/both the order and presence of the layers in the DOM?

@ahmadayubi
Copy link
Member

I think the reload was intended to just reset history and return to the initial location, it doesn't reset any other properties so I don't think it should reset the initial layers either.

@ahmadayubi
Copy link
Member

Also do you have the code for your mockup somewhere? It's a good UI implementation .

@Malvoz
Copy link
Member

Malvoz commented Jan 29, 2021

Also do you have the code for your mockup somewhere?

I don't, I was just messing around with it 😋 It'd probably need changes per #263 anyway.

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 a pull request may close this issue.

3 participants