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

feat: [M3-8170] - Add CheckoutBar Story #10784

Merged

Conversation

pmakode-akamai
Copy link
Contributor

@pmakode-akamai pmakode-akamai commented Aug 14, 2024

Description πŸ“

Add CheckoutBar Story to Storybook

Changes πŸ”„

List any change relevant to the reviewer.

  • Add CheckoutBar Story

Target release date πŸ—“οΈ

N/A

How to test πŸ§ͺ

Verification steps

  • Run yarn storybook to test locally

As an Author I have considered πŸ€”

Check all that apply

  • πŸ‘€ Doing a self review
  • ❔ Our contribution guidelines
  • 🀏 Splitting feature into small PRs
  • βž• Adding a changeset
  • πŸ§ͺ Providing/Improving test coverage
  • πŸ” Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • πŸ‘£ Providing comprehensive reproduction steps
  • πŸ“‘ Providing or updating our documentation
  • πŸ•› Scheduling a pair reviewing session
  • πŸ“± Providing mobile support
  • β™Ώ Providing accessibility support

@pmakode-akamai pmakode-akamai self-assigned this Aug 14, 2024
@pmakode-akamai pmakode-akamai requested a review from a team as a code owner August 14, 2024 15:04
@pmakode-akamai pmakode-akamai requested review from carrillo-erik and coliu-akamai and removed request for a team August 14, 2024 15:04
@pmakode-akamai pmakode-akamai marked this pull request as draft August 14, 2024 15:04
Copy link

github-actions bot commented Aug 14, 2024

Coverage Report: βœ…
Base Coverage: 82.63%
Current Coverage: 82.65%

@pmakode-akamai pmakode-akamai marked this pull request as ready for review August 19, 2024 06:35
@pmakode-akamai pmakode-akamai marked this pull request as draft August 19, 2024 06:35
@pmakode-akamai pmakode-akamai marked this pull request as ready for review August 19, 2024 11:56
Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

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

thanks for the PR! Had a few thoughts/comments

  • Could we change the text color of the children and agreement/footers? I can't see them unless I highlight the text. Maybe a color that works in both light/dark mode, or use the theme
    image

  • Could we add some documentation for the props of CheckoutBar so that we can see their descriptions in the story? For example (see corresponding documentation)
    image

  • Could we also add some tests for the CheckoutBar component?

We did a whole bunch of storybook migrations late last year - some of these PRs might be helpful to look at: here and here

@pmakode-akamai
Copy link
Contributor Author

pmakode-akamai commented Aug 20, 2024

  • Could we also add some tests for the CheckoutBar component?

Sure @coliu-akamai, but this PR is focused on Storybook. Could we have a separate ticket for adding tests to the CheckoutBar component?

Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

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

sure, we can go with a separate ticket for tests - would you be able to create one? Thanks for the changes + good work!

left one small comment below πŸ˜…

@coliu-akamai coliu-akamai added the Add'tl Approval Needed Waiting on another approval! label Aug 20, 2024
@pmakode-akamai
Copy link
Contributor Author

sure, we can go with a separate ticket for tests - would you be able to create one? Thanks for the changes + good work!

@coliu-akamai I have created a new ticket (M3-8463) for tests. Keeping this one as a todo..

@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Aug 22, 2024
@jaalah-akamai jaalah-akamai merged commit 43366ac into linode:develop Aug 23, 2024
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Storybook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants