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 a fullscreen parameter to display bigger menu in map applications #756

Merged
merged 8 commits into from
Mar 7, 2023

Conversation

12rambau
Copy link
Member

Fix #755

The idea is simle sometime I don't need the map anymore and I just want to display stuff to my users.
With this control option, I can create a control that fit nicely the frame of a map application (margin and icon width are absolute). It's only based on .css tricks so it cannot be coded outside of the lib.

let me know if you want to tune it. here are 2 examples, one in an app, one with a standalone fullscreen map:

Enregistrement de l’écran 2023-02-10 à 17 18 32

@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Merging #756 (0bcecd1) into main (2576446) will decrease coverage by 0.44%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #756      +/-   ##
==========================================
- Coverage   94.01%   93.57%   -0.44%     
==========================================
  Files          42       42              
  Lines        4508     4283     -225     
==========================================
- Hits         4238     4008     -230     
- Misses        270      275       +5     
Impacted Files Coverage Δ
sepal_ui/mapping/menu_control.py 100.00% <100.00%> (ø)
sepal_ui/conf.py 60.00% <0.00%> (-40.00%) ⬇️
sepal_ui/reclassify/table_view.py 58.25% <0.00%> (-1.48%) ⬇️
sepal_ui/aoi/aoi_model.py 87.87% <0.00%> (-1.32%) ⬇️
sepal_ui/reclassify/reclassify_view.py 83.53% <0.00%> (-1.02%) ⬇️
sepal_ui/reclassify/reclassify_model.py 91.83% <0.00%> (-0.62%) ⬇️
sepal_ui/mapping/legend_control.py 93.10% <0.00%> (-0.55%) ⬇️
sepal_ui/sepalwidgets/app.py 95.63% <0.00%> (-0.30%) ⬇️
sepal_ui/sepalwidgets/inputs.py 99.50% <0.00%> (-0.27%) ⬇️
sepal_ui/mapping/inspector_control.py 95.08% <0.00%> (-0.24%) ⬇️
... and 33 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@12rambau
Copy link
Member Author

The documentation bug is relative to this PR: sphinx-doc/sphinx#10691
Nothing to do from here we just need for this fix to be merged.

@12rambau 12rambau marked this pull request as ready for review February 13, 2023 11:57
Copy link
Collaborator

@dfguerrerom dfguerrerom left a comment

Choose a reason for hiding this comment

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

I like the idea, it looks great, I've tested it and works as expected.

  • What do you think if we add a centered button at the bottom of the card saying "close"? As the card renders over all the window, the user could lose the sense of where it was opened (because it is not at the corner of the map btn as the others)... or perhaps a cross icon at the top right corner?
  • As I understand the menu card, I think that the content will be used to display text, right? what about centering the title?

@12rambau
Copy link
Member Author

As I understand the menu card, I think that the content will be used to display text, right? what about centering the title?

I was more picturing a dashboard tile when you don't need the map anymore, let me send you a mockup as an example

What do you think if we add a centered button at the bottom of the card saying "close"? As the card renders over all the window, the user could lose the sense of where it was opened (because it is not at the corner of the map btn as the others)... or perhaps a cross icon at the top right corner?

let me ask the question on UX stackexchange. I would like to avoid the big button but I'm not sure people will notice little "x" in the top header. there is still the "escape" otpion as well

@12rambau
Copy link
Member Author

comming back from UX, they suggested to fix the state "active" of the menu btn while the window is opened so that you can recognize where you're coming from.

The x was considered but seems weird if not set everywhere. I'll go with that, update the tests and merge it

@12rambau 12rambau merged commit a20be94 into main Mar 7, 2023
@12rambau 12rambau deleted the fullscreen branch March 7, 2023 12:46
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.

create fullscreen menucontrol
2 participants