Skip to content

Cleanup and improvements to the Navigator #423

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

Conversation

dobromir-hristov
Copy link
Contributor

@dobromir-hristov dobromir-hristov commented Aug 25, 2022

Bug/issue #, if applicable: 97716711

Summary

This PR refactors the Navigator to make it more robust and cleans up the styling.

  1. Spacings to be configurable via CSS variables.
  2. Cleans up the sticky styling so its more robust
  3. Allows specifying the filter positioning via a prop
  4. Allow having none-truncated titles in nav items

Dependencies

NA

Testing

Steps:

  1. Make sure there are no styling regressions.
  2. Try to switch the filter positioning in the Vue dev tools by flipping the boolean prop on the Navigator.vue
  3. Try to scroll fast and see if there are perf issues
  4. Try in devtools to remove the whitespace: nowrap so titles overflow. Assert the navigator works as expected.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • Added tests
  • Ran npm test, and it succeeded
  • Updated documentation if necessary

@dobromir-hristov dobromir-hristov changed the title Cleanup and improvements to the Navigator sticky styling, spacings and filter positioning. Cleanup and improvements to the Navigator Aug 29, 2022
Dobromir Hristov added 2 commits August 29, 2022 19:13
…g to allow no truncation on navigator items.
…-css-var

# Conflicts:
#	tests/unit/components/Navigator/NavigatorCard.spec.js
@dobromir-hristov dobromir-hristov marked this pull request as ready for review September 8, 2022 07:44
Dobromir Hristov added 2 commits September 19, 2022 10:20
…-css-var

# Conflicts:
#	src/components/Navigator/NavigatorCard.vue
#	tests/unit/components/Navigator.spec.js
#	tests/unit/components/Navigator/NavigatorCard.spec.js
#	tests/unit/views/DocumentationTopic.spec.js
@dobromir-hristov
Copy link
Contributor Author

@swift-ci test

@dobromir-hristov
Copy link
Contributor Author

@swift-ci test

Copy link
Member

@marinaaisa marinaaisa left a comment

Choose a reason for hiding this comment

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

Left a question. Everything else looks good. I also tested it out

@dobromir-hristov
Copy link
Contributor Author

@swift-ci test

Copy link
Contributor

@mportiz08 mportiz08 left a comment

Choose a reason for hiding this comment

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

Changes generally look ok to me and I'm not seeing any obvious issues testing this for regressions.

@dobromir-hristov
Copy link
Contributor Author

@swift-ci test

@dobromir-hristov
Copy link
Contributor Author

@swift-ci test

@dobromir-hristov dobromir-hristov merged commit 3eb58b7 into swiftlang:main Sep 22, 2022
@dobromir-hristov dobromir-hristov deleted the dhristov/r97716711-move-navigator-spacing-to-css-var branch September 22, 2022 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants