Skip to content

Conversation

halocline
Copy link
Collaborator

@halocline halocline commented Aug 23, 2022

What does this PR do?

Closes #6276 by adding weight to <Heading />

Where should the reviewer start?

Heading.js

What testing has been done on this PR?

Jest test suite, Storybook, plus new snapshot.

How should this be manually tested?

<Heading weight="normal">My Heading</Heading>
<Heading weight="bold">My Heading</Heading>
<Heading weight={700}>My Heading</Heading>

Do Jest tests follow these best practices?

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • userEvent is used in place of fireEvent.
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

#6276

Screenshots (if appropriate)

Do the grommet docs need to be updated?

Yes.

Should this PR be mentioned in the release notes?

Yes.

Is this change backwards compatible or is it a breaking change?

Backwards compatible.

@halocline halocline requested a review from jcfilben August 23, 2022 15:43
Copy link
Collaborator

@MikeKingdom MikeKingdom 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. Minor indent comment...

Comment on lines +23 to +24
(data && data.maxWidth) ||
levelStyle.medium.maxWidth};
Copy link
Collaborator

Choose a reason for hiding this comment

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

could use the indent. It's a little hard to read which styles are involved

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmmm... let me see what I can do. I believe Prettier was forcing this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Something in the Prettier/ESLint space is forcing this onSave. I couldn't track down the rule / plugin causing this in a reasonable time. Okay with proceeding as is?

Comment on lines +26 to +27
levelStyle.font.weight ||
headingTheme.weight};
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

Copy link
Collaborator

@jcfilben jcfilben left a comment

Choose a reason for hiding this comment

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

Looks good! We will need a PR on grommet-site to update the docs

@ericsoderberghp ericsoderberghp merged commit 991846c into grommet:master Aug 23, 2022
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.

Heading — support weight and/or kind
4 participants