Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Play-311] - Add Sidebar For Visual Guidelines React Page #2054

Merged
merged 3 commits into from
Sep 10, 2022

Conversation

nidaqg
Copy link
Contributor

@nidaqg nidaqg commented Sep 7, 2022

Screens

Screen Shot 2022-09-07 at 11 30 28 AM

Screen Shot 2022-09-07 at 11 30 48 AM

Breaking Changes

No breaking changes, adding to playbook website only

Runway Ticket URL

Runway Story

How to test this

Test in milano env

Checklist:

  • LABELS Add a label: enhancement, bug, improvement, new kit, deprecated, or breaking. See Changelog & Labels for details.
  • DEPLOY Please add the Milano label when you are ready for a review.
  • SCREENSHOT Please add a screen shot or two.
  • SPECS Please cover your changes with specs.
  • READ DOCS Please make sure you have read and understand the Playbook Release Process

@nidaqg nidaqg added milano 20 MAX - Deploy this PR to a review environment via Milano improvement This is used when your PR contains library upgrades or doc/site improvements. (USED IN CHANGELOG)) labels Sep 7, 2022
@nidaqg nidaqg requested a review from a team as a code owner September 7, 2022 15:33
Copy link
Contributor

@thestephenmarshall thestephenmarshall left a comment

Choose a reason for hiding this comment

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

Just one comment. It is not really critical, but just wanted to better understand.

"Flex Box",
];

const ReactSidebar = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we just called this Sidebar? I don't know why we would need to add React to the naming unless I'm missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will make that change 👍🏽


export default VisualGuidelines
export default VisualGuidelines;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for converting the singles to doubles and adding those anchor links! 🚀

<% end %>

<% if current_page?('/visual_guidelines_react') %>
<%= react_component("ReactSidebar") %>
<% end %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<% end %>
<% end %>

Copy link
Contributor

@augustomallmann augustomallmann left a comment

Choose a reason for hiding this comment

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

Great job Nida!!

@jasperfurniss jasperfurniss added the Ready for Release merged to master, ready for a versioned released label Sep 9, 2022
@jasperfurniss jasperfurniss merged commit 2fa261d into master Sep 10, 2022
@jasperfurniss jasperfurniss deleted the PLAY-311-Sidebar-Visual-Guidelines-React branch September 10, 2022 14:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
improvement This is used when your PR contains library upgrades or doc/site improvements. (USED IN CHANGELOG)) milano 20 MAX - Deploy this PR to a review environment via Milano Ready for Release merged to master, ready for a versioned released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants