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

Add hints and feedback for optional layers tree usage #263

Merged
merged 3 commits into from
Dec 6, 2019

Conversation

nrenner
Copy link
Owner

@nrenner nrenner commented Nov 28, 2019

An attempt to make usage of the optional layers tree more clear, see #211, while keeping the behavior for now:

  • help message on top (using button tooltip): "Add or remove optional layers"
  • icon tooltips as hints and feedback for add/remove and preview actions
  • "Preview" label on the map
  • lower opacity and default cursor for disabled root node checkboxes

@Phyks what do you think?

@Phyks
Copy link
Contributor

Phyks commented Nov 30, 2019

Looks really nice! Can you only show the "+ / -" icon tooltips when hovering the checkbox?

@nrenner
Copy link
Owner Author

nrenner commented Nov 30, 2019

I could, this is where I started, but apparently not the preview tooltip when only hovering the text part, as the text is not wrapped in an element:

<a class="jstree-anchor jstree-checked" href="#" tabindex="-1" id="standard_anchor">
  <i class="jstree-icon jstree-checkbox" role="presentation"></i>
  <i class="jstree-icon jstree-themeicon" role="presentation"></i>
  OpenStreetMap
</a>

CSS select for the checkbox in the browser console:

document.querySelector('.jstree-default .jstree-checked .jstree-checkbox')

But text only seems not to be selectable in CSS (?):
Is there a CSS selector for text nodes? - Stack Overflow

And I don't want to get into overriding the text rendering in JSTree right now:
Wrap text of <a> (jstree-anchor) into <span> · Issue #1930 · vakata/jstree

So I thought it would make sense and be more consistent to have them both behave the same way? And I guess always showing both is also a better immediate hint at the two different actions for an entry?

@Phyks
Copy link
Contributor

Phyks commented Dec 1, 2019

But text only seems not to be selectable in CSS (?):

You probably cannot do it from within the querySelector call (no CSS selector for text nodes), but you might be able to post-filter it. See https://stackoverflow.com/a/10730777 for instance (if using JS here is fine).

And I don't want to get into overriding the text rendering in JSTree right now:

👍

So I thought it would make sense and be more consistent to have them both behave the same way? And I guess always showing both is also a better immediate hint at the two different actions for an entry?

In case there is an easy way to do it, I think this would be a nice improvement. But definitely 👍 for me for not spending too much time hacking around JSTree so current behavior is fine as well.

@nrenner
Copy link
Owner Author

nrenner commented Dec 2, 2019

Hovering is now separate for checkbox and text.

As the text property accepts HTML (and I actually already use this for the language/country code prefix), wrapping it in an element was actually quite simple, no need for a plugin.

@nrenner nrenner merged commit a71dfe7 into master Dec 6, 2019
@nrenner nrenner deleted the 211-layer-tree-hints branch December 6, 2019 08:32
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.

2 participants