-
Notifications
You must be signed in to change notification settings - Fork 55
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
Style picker for client stylin #1510
Style picker for client stylin #1510
Conversation
…to origo-master-feature-alternative-style
Just to clarify, this works for vector layer (WFS, geojson) with standard client styling. Doesn't work with group layers or cluster styling. |
@Polleryd Please edit your comment to add a magic word before the issue number in order to link the PR to the issue: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @johnnyblasta, a label would be nice. Also, the blue ring feels a bit too much. It's not immediately clear what it means and that might be a bit confusing for the average user. |
Nice, at least minus the blue ring. In addition the style picker appears to fail after the Print control has been opened and closed. I don't see an error in the console but for the default origo cities layer switching between styles efter Print control exit doesn't appear to do anything anymore. The better news is that #1514 appears to fix that in my brief and preliminary test. Additionally it would be nice if the selected style was remembered in a mapstate (sharemap control storeMethod option set to saveStateToServer) |
The main reason we added the blue ring around the style icon is to tell the user "Hey, you can choose a different style on this layer". Our intention is to use stylepicker for about 15% of the layers which means that we wanted to help the user to know which layers you can change style on. Without any indicator, it will be very clicky to find out if a layer has a style picker. What about making it to an option under the legend control like "stylepickerIndicator": true/false? The indicator can be changed later. Is that ok? Does anyone have a suggestion for another indicator? Regarding label for the drop-down list, you can choose to clarify through the title for each style such as "Style: Origo logo". Then the user understands what the dropdown does and the layer information stays clean. But that's just my opinion. I guess we can add a label, since you want it. @Grammostola, thank you for testing this one with the print control! We'll have a look at mapstate. |
@asemoller I also object to the blue ring. It would only clutter up the GUI. Soon someone else will want a yellow underscore to indicate that the layer is queryable, a red square to indicate that data is only accurate on thursdays etc... The users will soon learn which layers must/can be changed when background changes. Or else they can always try the three dot menu and see what is possible to do with each layer. Most likely there will appear more stuff there (like zoom to extent) and the options will differ from layer to layer so the users will soon get a habit of try that menu to see what is possible to do for each layer. |
As a did take on the role of accessibility "chief" I have to point out that the dropdown style button is missing aria-label or aria-labelledby (which should point to the missing label if added) |
ok, we got it! 😄 We will do this:
|
@steff-o made a good point regarding GUI clutter. And also, blue ring or not, you'd still have to tell the user that there are multiple styles available, either by explaining the meaning of the blue ring or by saying that some layers may have multiple styles, click the More button and find out if you think a layer isn't clearly visible. Just a blue ring is not very intuitive. That said, I'm not completely against hints like this, @asemoller. I just don't think there's a point having it unless it's somewhat clear what it means. Especially in this case since it "changes" the look of the layer symbol for no obvious reason. So, I'm inclined to say that the blue ring is something you should implement locally instead of adding it to the core. But I also realize that others might find it useful is some cases, so perhaps it could be a configurable feature, off by default. But you'd have to make sure it doesn't eff up the styling of the rest of the legend by pushing pixels around, and that it works well with the tab-index stuff. |
@tonnyandersson we'll remove it, no worries! In fact, some icons are not completely in the middle and that disturbs the symmetry 😮 and that's never good! |
…e-style-pr-fixes Origo master feature alternative style pr fixes
We have fixed all the points listed above (including mapstate). And it also works well to change one cluster style to another cluster style using "clusterStyle" in the stylePicker like this
|
@Grammostola Have you had a chance to test the mapstate thing since the latest update? |
Yes, wfs and geojson and simpler styles at least, worked well. |
There are still some wrong use of accessibility tags. |
@johnnyblasta Is there any good example in origo that we can look at? |
It isn't necessary to use aria-labelledby if aria-label is used, but if you do use aria-labelledby it will take precedence and then it should be used correctly. There isn't a example in Origo master yet , but I have a example of aria-labelledby on my own fork |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Resolves #1020
In the configuration file, add the field "stylePicker" to the layer. It should be an array with the styles available for the user to choose from on that layer.
"stylePicker": [
{"title": "Title_1", "style": "style_1"},
{"title": "Title_2", "style": "style_2"}
],