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

feat: Reset route if neither the Viewer of the Sidebar is open #47920

Merged

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Sep 12, 2024

When the viewer or the sidebar is opened, we add the fileid to the route.
When both of them are closed, we do not remove the fileid from the route.
This means that, upon reload, the sidebar will be opened even though it was closed previously.

This PR ensure that the fileid is removed from the route when both the Sidebar and the Viewer are closed.

Screencast.from.2024-09-12.15-38-42.mp4

@artonge artonge self-assigned this Sep 12, 2024
@artonge artonge requested a review from susnux September 12, 2024 12:35
@artonge artonge added 3. to review Waiting for reviews feature: files javascript feature: file sidebar Related to the file sidebar component labels Sep 12, 2024
@artonge artonge marked this pull request as draft September 12, 2024 12:36
@artonge artonge added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Sep 12, 2024
@artonge artonge force-pushed the artonge/feat/reset_route_when_sidebar_and_viewer_are_close branch from 0bc7e5e to 87927e5 Compare September 12, 2024 13:39
@artonge artonge marked this pull request as ready for review September 12, 2024 13:44
@artonge artonge force-pushed the artonge/feat/reset_route_when_sidebar_and_viewer_are_close branch 2 times, most recently from 9998b21 to fc91cf3 Compare September 12, 2024 13:51
Comment on lines 275 to 283
// If the Sidebar is closed and if openFile is false, remove the file id from the URL
if (OCA.Files.Sidebar.file === '') {
window.OCP.Files.Router.goToRoute(
null,
{ ...this.$route.params, fileid: undefined },
this.$route.query,
)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not exactly a Viewer close event, but does the job.

@artonge artonge force-pushed the artonge/feat/reset_route_when_sidebar_and_viewer_are_close branch from fc91cf3 to 1800d87 Compare September 12, 2024 14:39
@artonge artonge requested a review from susnux September 12, 2024 16:09
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

makes sense (ideally add a cypress tests), only thing I am not sure about if this really should be inside the virtual file list. But code makes sense and seems to work fine.

apps/files/src/components/FilesListVirtual.vue Outdated Show resolved Hide resolved
@artonge artonge force-pushed the artonge/feat/reset_route_when_sidebar_and_viewer_are_close branch from 1800d87 to 11ff3d0 Compare September 13, 2024 08:46
@artonge artonge requested a review from susnux September 13, 2024 08:47
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Wait for it

@artonge artonge force-pushed the artonge/feat/reset_route_when_sidebar_and_viewer_are_close branch from 11ff3d0 to bb2d8e1 Compare September 14, 2024 19:38
When the viewer or the sidebar is opened, we add the fileid to the route.
When both of them are closed, we do not remove the fileid from the route.
This means that, upon reload, the sidebar will be opened even though it was closed previously.

This PR ensure that the fileid is removed from the route when both the Sidebar and the Viewer are closed.

Signed-off-by: Louis Chemineau <[email protected]>
@artonge artonge force-pushed the artonge/feat/reset_route_when_sidebar_and_viewer_are_close branch from bb2d8e1 to 2da1c57 Compare September 14, 2024 19:50
@artonge artonge merged commit 3c9111e into master Sep 14, 2024
116 checks passed
@artonge artonge deleted the artonge/feat/reset_route_when_sidebar_and_viewer_are_close branch September 14, 2024 21:49
@skjnldsv
Copy link
Member

Backports ?

@artonge
Copy link
Contributor Author

artonge commented Sep 15, 2024

/backport to stable30

@artonge
Copy link
Contributor Author

artonge commented Sep 15, 2024

/backport to stable29

@artonge
Copy link
Contributor Author

artonge commented Sep 15, 2024

/backport to stable28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants