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

[EuiDataGrid] Provide a way to close the fullscreen view with the browser's back button #4393

Closed
kertal opened this issue Dec 16, 2020 · 9 comments

Comments

@kertal
Copy link
Member

kertal commented Dec 16, 2020

Started discussion in elastic/kibana#67259 (comment)
@majagrubic :

When selecting the fullscreen option from the menu and clicking Back in the browser, I'm taken back to the Advanced Settings, ie. the last page I was on before Discover. I would expect this to take me back to Discover.

@chandlerprall

Would need to hook into the html5 history api for that. I don't know if that's functionality we'd want in EUI, though it seems like it so it would be consistent between all fullscreen grids.

so let's start to discuss

@cchaos
Copy link
Contributor

cchaos commented Dec 16, 2020

Just some 2 cents here in terms of UX. One of the deepest problems SPA's has is not utilizing the browser history enough. So I'm all for this, however you want to handle it technically, which most likely is not built into EUI but provides some sort of callback for applications to handle it.

@j-m
Copy link
Contributor

j-m commented Dec 17, 2020

To put in my two-penny worth, too, fullscreen of a datagrid is akin to a modal or, at a stretch, a flyout. If fullscreen were to be tracked in history, then I'd expect modals and flyouts to be, too. I think a feature like this has further implications than just datagrid

@myasonik
Copy link
Contributor

🤔 This is interesting because I think I agree with both of you... So here's my rational:

Going to full-screen with datagrid (or anywhere else we allow it), I think, is not the same as semantically moving to another page. In this way, it shouldn't be in the browser history.

But, because it does obstruct the other page completely (as opposed to modals or flyouts) it's easy to forget how you got to where you are. I think there's a level of muscle memory in browsers of "if you got somewhere you don't want to be, hit 'back'". And if someone does that in a full-screen context, it will then drop them too far back.

So, though I think technically it's not what the back button is for, I think it ultimately does help the user if we did use it here. With back button support, the worst case for a user who was carefully following their browser history is that they have to click back an extra time. Without back button support, the worst case for a user who mistakenly thought they were on some new page, is losing state. Losing state seems like a much worse "worst case" than an extra button press.

@chandlerprall
Copy link
Contributor

I think whichever way we decide, it will certainly upset some folks. My strong opinion is that back/forward should always be tied to URL changes representing the page's state. EUI should never modify or otherwise impact the URL or navigation actions as it is not our property. I think the best option is for datagrid to provide a callback for when fullscreen is entered/exited, and a way for the application to set it. This enables:

  • applications can opt to close fullscreen on back navigation, or allow the default browser action
  • deep linking to a full screen grid experience

Exposing the fullscreen state + allowing an application to set it came up recently in an internal conversation which also included these same changes for a grid's density. This does not, however, solve uniformity between implementations.

@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@kertal
Copy link
Member Author

kertal commented Jun 17, 2021

think we should not close it , although it has not the highest priority :)

@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@cchaos
Copy link
Contributor

cchaos commented Dec 14, 2021

This is a major problem with any full screen functionality especially if users who use the back button could encounter data loss. I'm forever skipping stale check.

@daveyholler
Copy link
Contributor

Closing as not planned. There's been some good arguments laid out in this issue. All make valid points, however after careful consideration we have decided that this is not a interaction we want to introduce to EUI.

@daveyholler daveyholler closed this as not planned Won't fix, can't repro, duplicate, stale Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants