[docs]: fix the RightPanel to work correctly with browser history. #1166
Conversation
|
Tested on https://dvc-landing-right-panel-bsvd42.herokuapp.com/doc/command-reference/checkout and this works, thanks! I'd approve it but it sounds like Ivan has some other feedback. |
Thanks! @shcheklein I have updated the Pr accordingly. Please have a look whenever you'll have time. |
|
@nisarhassan12 I still do have some strange behavior for the right panel after going back - try to open https://dvc-landing-right-panel-lul3q9.herokuapp.com/doc/command-reference/diff#example-checking-workspace-changes , open a few collapsible sections there, click to some link, go back and use the right content panel. |
|
Also, what will we be getting in IE 11? |
@shcheklein According to MDN I'm sorry I can't test in an actual IE 11 because I use Arch as my OS. but let say if a browser is very old and does not support localStorage then state of the collapsed panel won't be persisted although the back button would take you to the exact location. A bit un-related: the site currently uses CSS custom properties which are not supported at all in all versions of IE and old versions Edge( source: https://caniuse.com/#feat=css-variables ) I would recommend not using them if you want to support IE and other older browsers.
|
|
@nisarhassan12 thanks! IE is no a top-top priority - the only thing we want to avoid is a complete crash on it and Sentry complaining. Re this PR - please take a look here #1166 (comment) . |
Thanks! @shcheklein I'll try to fix this. |
|
|
||
| exports.wrapPageElement = PageWrapper | ||
|
|
||
| exports.onPreRouteUpdate = ({ location, prevLocation }) => { |
There was a problem hiding this comment.
can we make collapsible itself saving this on the expnasion? don't like that we do this global search on every navigate
| let details = [] | ||
| try { | ||
| details = JSON.parse(localStorage.getItem('details') || '[]') | ||
| } catch {} |
There was a problem hiding this comment.
silent catch is bad? what kind of errors do we expect here?
There was a problem hiding this comment.
an error can occur when localStorage is not supported or it fails to parse what is coming back from localStorage which can happen if someone goes into localStorage and set the value for the key to something like {, ']' and so on i.e when there is invalid JSON.
Since we are returning the empty array no matter what the app won't break.
I think adding a console.log would be helpful.
| const text = filteredChildren[0].props.children[0] | ||
| let details = getDetails() | ||
| const detailTitle = filteredChildren[0].props.children[0] | ||
| const detailText = getText(filteredChildren.slice(1)) |
There was a problem hiding this comment.
we need a better way to determine the specific collapsible. collecting the whole text feels too much, expensive.
|
I'm closing this PR as it does not fixes the issue and would add complexities to the codebase. |
Fixes #1140