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.

Sign up for GitHub

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

Jump to bottom

DSD-1603: component changelogs #1436

Merged
merged 7 commits into from
Oct 11, 2023
Merged

DSD-1603: component changelogs #1436

merged 7 commits into from
Oct 11, 2023

Conversation

bigfishdesign13
Copy link
Collaborator

Fixes JIRA ticket DSD-1603

This PR does the following:

  • Adds the ComponentChangelogTable component.
  • Adds a changelog to the story pages for the DatePicker, FeedbackBox, Hero, and Slider components.

How has this been tested?

  • local Storybook

Accessibility concerns or updates

  • n/a

Checklist:

  • I have updated the Storybook documentation accordingly.
  • [] I have added relevant accessibility documentation for this pull request.
  • All new and existing tests passed.

Front End Review:

  • Review the Vercel preview deployment once it is ready.

@vercel
Copy link

vercel bot commented Oct 6, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nypl-design-system ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 11, 2023 8:30pm

@bigfishdesign13 bigfishdesign13 added the Needs Review Pull requests that are ready for peer review. label Oct 6, 2023
@bigfishdesign13
Copy link
Collaborator Author

I flagged everyone on this PR so we're all on the same page going forward. As we update components (or add them anew), we will add and update a changelog for each individual component. Refer to the story pages that I've updated in this PR.

@aarnold101 and @7emansell – Once this is merged into development, you can pull it and then go ahead and add the appropriate code to your new components.

EdwinGuzman
EdwinGuzman requested changes Oct 10, 2023
View reviewed changes
Copy link
Member

@EdwinGuzman EdwinGuzman left a comment

Choose a reason for hiding this comment

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

Some updates but really like how it looks. As we add more updates in the changelog, the .mdx files will grow long and unmaintainable. I'm thinking we can have an additional file called something like [component]-changelog.ts that exports its own changelogData array for the ComponentChangelogTable component.

So for DatePicker, it would be DatePicker-changelog.ts. What do you think?

src/utils/ComponentChangelogTable.tsx Outdated Show resolved Hide resolved
src/utils/ComponentChangelogTable.tsx Outdated Show resolved Hide resolved
src/utils/ComponentChangelogTable.tsx Outdated Show resolved Hide resolved
src/utils/ComponentChangelogTable.tsx Outdated Show resolved Hide resolved
src/utils/ComponentChangelogTable.tsx Outdated Show resolved Hide resolved
@EdwinGuzman EdwinGuzman added the Changes Requested Pull requests where changes have been requested in peer review. label Oct 10, 2023
@bigfishdesign13 bigfishdesign13 removed the Changes Requested Pull requests where changes have been requested in peer review. label Oct 10, 2023
EdwinGuzman
EdwinGuzman reviewed Oct 10, 2023
View reviewed changes
src/components/DatePicker/datePickerChangelogData.ts Outdated
* affects: "Accessibility" | "Documentation" | "Functionality" | "Styles";
* notes: array (will render as a bulleted list, add one array element for each list element)
*/
export const changelogData = [
Copy link
Member

Choose a reason for hiding this comment

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

You can define the type to ChangelogData since that's already declared and could be used here to give better guidance for the type and affects values.

EdwinGuzman
EdwinGuzman approved these changes Oct 10, 2023
View reviewed changes
Copy link
Member

@EdwinGuzman EdwinGuzman left a comment

Choose a reason for hiding this comment

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

The update looks good. The only minor comment is to type the changelogData arrays in each data file so it's easier for devs to know what each property expects.

@bigfishdesign13
Copy link
Collaborator Author

@EdwinGuzman I think the changes in my last update were significant enough to necessitate one more review.

EdwinGuzman
EdwinGuzman reviewed Oct 11, 2023
View reviewed changes
src/utils/ComponentChangelogTable.tsx Outdated
borderColor: borderColor,
fontSize: "desktop.caption",
// pb: "xs",
// pt: "xs",
Copy link
Member

Choose a reason for hiding this comment

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

Okay to remove.

7emansell
7emansell approved these changes Oct 11, 2023
View reviewed changes
Copy link
Collaborator

@7emansell 7emansell left a comment

Choose a reason for hiding this comment

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

Looks great to me

@bigfishdesign13 bigfishdesign13 merged commit bdd99c5 into development Oct 11, 2023
4 checks passed
@bigfishdesign13 bigfishdesign13 deleted the DSD-1603/component-changelogs branch October 11, 2023 20:34
@bigfishdesign13 bigfishdesign13 added Ship It Pull requests that have been reviewed and approved. and removed Needs Review Pull requests that are ready for peer review. labels Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@7emansell 7emansell 7emansell approved these changes

@EdwinGuzman EdwinGuzman EdwinGuzman approved these changes

@jackiequach jackiequach Awaiting requested review from jackiequach

@oliviawongnyc oliviawongnyc Awaiting requested review from oliviawongnyc

@dgcohen dgcohen Awaiting requested review from dgcohen

@aarnold101 aarnold101 Awaiting requested review from aarnold101

@isastettler isastettler Awaiting requested review from isastettler

Assignees
No one assigned
Labels
Ship It Pull requests that have been reviewed and approved.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bigfishdesign13 @EdwinGuzman @7emansell