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
feat: implement fullscreen context + component #237
Conversation
✅ Deploy Preview for analysis-ui-components ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Codecov ReportBase: 91.85% // Head: 91.85% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #237 +/- ##
=======================================
Coverage 91.85% 91.85%
=======================================
Files 8 8
Lines 491 491
Branches 80 80
=======================================
Hits 451 451
Misses 40 40 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
I will try to find out whats the problem |
@lpatiny fullscreen bug fixed |
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.
Please create an example with 2 nested parts of the UI which can be toggled to fullscreen.
I meant 1 example which nests 2 fullscreen elements, not 2 examples which each have one fullscreen element. |
done, sorry for the misunderstanding |
"Nest" means 1 contains the other 😅 <FullScreenProvider>
<Content i="1" />
<FullScreenProvider>
<Content i="2" />
</FullScreenProvider>
</FullScreenProvider> |
^ @stropitek do you expect to be able to nest the fullscreen action? Meaning if you put the parent in full screen, you can then put the child too, ending up with two elements in full screen. |
Implemented. |
I see nesting as a potentially valid use case (for example full app + spectra area) and I wanted to see how it behaves with the current implementation. Apparently it does not behave correctly, but I'm not expecting that it works in this PR. I guess we need to validate that nesting / stacking is a valid use case. @lpatiny any thoughts? |
I think it make sense that many parts of the same application can go full screen and they could be nested but I think if we are in full screen we should not be able to put in full screen a part of it. This means that all the 'full screen' button should disappear once we are in full screen. I'm not aware of any application in which we can have a stack of full screen and this could be quite confusing for the end user. |
This behavior is weird if you "Toggle fullscreen 1" and the try to "Toggle fullscreen 2".
Why not? I think it should either replace the existing full screen element or create a stack, but not remove buttons (that would be difficult to implement) |
I fixed the behavior and it works perfectly now |
I think there's still an issue when the user exits fullscreen using the Escape key. In that case, you need to click twice on the button to enter fullscreen again. |
closes #217