Tabs for the new Display Package page redesign #8625
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
loic-sharma
reviewed
Jun 9, 2021
loic-sharma
reviewed
Jun 9, 2021
loic-sharma
reviewed
Jun 9, 2021
loic-sharma
reviewed
Jun 9, 2021
loic-sharma
reviewed
Jun 9, 2021
loic-sharma
reviewed
Jun 9, 2021
loic-sharma
reviewed
Jun 9, 2021
zhhyu
reviewed
Jun 9, 2021
zhhyu
reviewed
Jun 10, 2021
| @@ -334,6 +334,11 @@ img.reserved-indicator-icon { | |||
| .sortable { | |||
| cursor: pointer; | |||
| } | |||
| @media (max-width: 767.9px) { | |||
| .hidden-xs { | |||
| display: none !important; | |||
There was a problem hiding this comment.
Is it expected that we introduce this change? My apologies that I am not a less/css expert.
There was a problem hiding this comment.
This file is auto-generated by grunt. To be honest, it is not clear why grunt chose to add this now, but it seems to be okay regardless. I tried deleting the file and running grunt again to recreate it and it gets added every time
zhhyu
reviewed
Jun 10, 2021
|
I think we may need to polish the accessibility later, but it's out of scope of this PR. |
zhhyu
approved these changes
Jun 10, 2021
loic-sharma
reviewed
Jun 11, 2021
loic-sharma
reviewed
Jun 11, 2021
loic-sharma
reviewed
Jun 11, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Solution
I used bootstrap to created tabs to match the new redesign. The tabs were made sticky the same way that the installation tabs are by adding code in the .js file that saves the last tab clicked onto local storage. For styling, I went into the .less file and added the blue bar at the bottom of the clicked on tab and added padding to keep the distance of the tab content to the tabs consistent for each content. The final now looks like this:
Mobile
The mobile is still a work in progress as we are finalizing how we want it to look to keep the experience just as strong as it will be on desktop. All the features and experiences are the same currently and the only issue is the tabs in the bottom row get pushed if a tab in the top row is clicked(see picture below). We decided to punt this as we have a work item(#8611) for mobile and want to wait until the redesign is finalized to make changes.
Testing
To test my solution I tested all the links on both the new page and old page to make sure that it all worked correctly. I then inspected the page and looked at the console to make sure that there were no errors in the .js file. I also double checked that the sticky tabs worked by reloading the page, going to a different package, and seeing that the correct value was being stored in local storage. Finally, to make sure that the page worked on deleted and unlisted packages I deleted and unlisted packages in my local enviroment. I then looked at them to make sure that only the used by and versions tabs were shown to match what we currently do on the old page.
If you have any questions feel free to reach out