Skip to content

fix(preview-sidebar): fix version not updating on small screens#3095

Open
greathmaster wants to merge 1 commit into
box:masterfrom
greathmaster:fix-resp-preview-version-beahavior
Open

fix(preview-sidebar): fix version not updating on small screens#3095
greathmaster wants to merge 1 commit into
box:masterfrom
greathmaster:fix-resp-preview-version-beahavior

Conversation

@greathmaster

Copy link
Copy Markdown
Contributor

Overview

Currently, if a version is selected in the Preview sidebar and the Preview sidebar is closed, it reverts to the latest version of the document.

This creates problems for small screens where the Preview sidebar is moved to the bottom and becomes a vertical sliding drawer. When an old version is selected and the drawer is closed, a user cannot view the old version because, it reverts to the latest version of the document.

This PR fixes the issue by removing the version reset performed when the drawer is closed.

Note this would also affect the regular desktop experience. If a user closes the Preview Sidebar after selecting the version, it would remain on that version instead of changing back to the latest version.

Demo

Before

Preview-Drawer-Before

After

Preview-Drawer-After

@jstoffan

Copy link
Copy Markdown
Contributor

@greathmaster, has the desktop change been approved by the relevant stakeholders?

@greathmaster greathmaster left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've asked the relevant stakeholders and they are okay with the the behavior change on desktop experiences.

}
}

componentWillUnmount() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey so are there any possible side effects of removing this code completely. If we can remove it then it looks like it may have been unnecessary code in the first place.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This code was necessary to support updating to the latest preview version after closing the sidebar.

The change updates the behavior to remain on the version that was selected after closing the sidebar.

So it was needed before, but with the change in behavior it is not needed anymore. I don't believe it will have any side effects.

@jstoffan Do you see any potential side effects with this change?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It also means that switching to other sidebars, like "Details" or "Metadata", will not revert to the current version preview. The only mechanism to do so will be the button in the header.

Have we heard back from the relevant stakeholders that this change is acceptable even on larger screen sizes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Have approval for the version behavior change, ie to show the previous version after closing the right sidebar on large screens. However, let me circle back after checking about the "Details" and "Metadata" experiences.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@greathmaster, understood. I would think the Next/Previous buttons to navigate forward and backward through previews in a folder may also be affected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants