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

UITOOL-322 reorganize routes #2309

Merged
merged 46 commits into from Dec 1, 2021
Merged

Conversation

j-mcgregor
Copy link
Contributor

@j-mcgregor j-mcgregor commented Nov 22, 2021

Proposed Changes

Key Changes

  • demo/src/components & demo/src/styles are deleted
  • New routes can be found in demo/src/plasma
  • Navigation is hardcoded using nested Switches and Routes
    • I've tried to organize routes in a way that will make upgrading to React Router v6 easier
    • Navigation also uses context as its a more efficient way of accessing the pathname than 65+ useLocations or props drilling on every component
  • Routes are organized under their corresponding sub navigation categories (layout, navigation, foundations etc)
  • useMarkdown hook added in each page with corresponding src/plasma/utils/MarkdownDocs.ts imports file for easy-to-add Markdown documentation inside VaporComponent. Pages without Markdown documentation contain a link to Github as per UITOOL-332
Screen.Recording.2021-11-26.at.11.25.09.AM.mov

VaporComponent

  • Good contender to be refactored and used as the Page Component
  • useCodeExample hook in VaporComponent

New Layout

- src
    - plasma
        - navigation
            - SideNavigation
        - routes
            - display-and-utilities
                - pages
                  - BordersExamples.tsx
                  - ...
                - index.tsx
            - feedback-and-info
            - foundation
            - input
            - layout
            - navigation
            - legacy (all other components that weren't included in the mockup)
        - utils
        Brand page
        Homepage

I would suggest checking out this branch locally as the structural changes will be easier to see

Potential Breaking Changes

Acceptance Criteria

  • The proposed changes are covered by unit tests
  • The potential breaking changes are clearly identified
  • README.md is adjusted to reflect the proposed changes (if relevant)

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

@j-mcgregor j-mcgregor marked this pull request as ready for review November 25, 2021 14:03
@j-mcgregor j-mcgregor requested a review from a team as a code owner November 25, 2021 14:03
@j-mcgregor j-mcgregor requested review from a team, mikedidomizio, gdostie, FelixBlaisThon and aiheon and removed request for a team November 25, 2021 14:03
Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

@j-mcgregor j-mcgregor changed the title UI tool 322 reorganize routes UITOOL-322 reorganize routes Nov 25, 2021
@aiheon
Copy link
Collaborator

aiheon commented Nov 25, 2021

I have one thing I noticed here 🧐

Peek.2021-11-25.10-38.mp4

@j-mcgregor
Copy link
Contributor Author

I have one thing I noticed here 🧐

Peek.2021-11-25.10-38.mp4

oops, wrapper's in the wrong place, thanks

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

Jack added 22 commits November 30, 2021 16:05
Every page now has a vapor component that takes in markdown from a hook and local md files
moved the necessary functionality to make code blocks for examples in separate file
Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

Stale review, a new push occured on the branch. Check for the new status ⬇️

@toofff
Copy link
Member

toofff commented Dec 1, 2021

  1. In the section Radio buttons is down for example => https://vapor.coveo.com/feature/PR-2309/index.html#/input/RadioButton

  2. In the section Value Popup this difficult for read this example => https://vapor.coveo.com/feature/PR-2309/index.html#/input/ValuePopup

image

  1. In the section SubNavigation is down for example => https://vapor.coveo.com/feature/PR-2309/index.html#/navigation/Subnavigation

  2. In the section List Box => https://vapor.coveo.com/feature/PR-2309/index.html#/legacy/ListBox

  • duplicate example List Box Connected
  • the example for List Box Connected is between the visual of List Box and the example of List Box
  1. In the section Icons list => https://vapor.coveo.com/feature/PR-2309/index.html#/foundations/Iconography
    No example is available, is this normal?

Copy link
Member

@toofff toofff left a comment

Choose a reason for hiding this comment

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

After small fix is OK for me ;)

Copy link
Contributor

@jenkins-dev-coveo jenkins-dev-coveo bot left a comment

Choose a reason for hiding this comment

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

@j-mcgregor j-mcgregor merged commit d80706f into master Dec 1, 2021
@j-mcgregor j-mcgregor deleted the UITOOL-322-reorganize-routes branch December 1, 2021 16:28
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.

None yet

4 participants