E539 feat(hint): added hasNoOffset prop by evwilkin · Pull Request #10488 · patternfly/patternfly-react · GitHub
[go: up one dir, main page]

Skip to content

feat(hint): added hasNoOffset prop#10488

Merged
tlabaj merged 9 commits intopatternfly:v6from
evwilkin:feat/10485-hint-offset
Jun 5, 2024
Merged

feat(hint): added hasNoOffset prop#10488
tlabaj merged 9 commits intopatternfly:v6from
evwilkin:feat/10485-hint-offset

Conversation

@evwilkin
Copy link
Member

What: Closes #10485

Additional issues: Follow-up to patternfly/patternfly#6708

This PR:

  • bumps the core version
  • adds support & tests for hasNoOffset prop on the Hint component
  • updates the existing actions behavior to only conditionally render the div.pf-v6-c-hint__actions if the actions prop is passed

@evwilkin evwilkin linked an issue May 31, 2024 that may be closed by this pull request
@patternfly-build
Copy link
Collaborator
patternfly-build commented May 31, 2024

@evwilkin evwilkin requested review from a team, kmcfaul and tlabaj and removed request for a team June 3, 2024 13:45
@tlabaj tlabaj requested a review from mcoker June 3, 2024 19:55
Copy link
Contributor
@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

LGTM! One comment would just be that where the prop goes is different between the card and hint and if it goes on the top-level component, I wonder if the prop name should reference "actions" (eg, hasNoActionsOffset).

Copy link
Contributor
@edonehoo edonehoo left a comment

Choose a reason for hiding this comment

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

some ideas! lmk what you think

@evwilkin evwilkin requested a review from edonehoo June 4, 2024 15:05
Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com>
@edonehoo edonehoo self-requested a review June 4, 2024 18:46
expect(asFragment()).toMatchSnapshot();
});

test('actions are rendered', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe verify here that the no offset style is not applied. Or add a new assertion.

Copy link
Member Author

Choose a reason for hiding this comment

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

@tlabaj I updated this test, does that work?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good!

@tlabaj
Copy link
Contributor
tlabaj commented Jun 5, 2024

Just needs a rebase.

@tlabaj tlabaj merged commit 42c42ad into patternfly:v6 Jun 5, 2024
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.

Hint - add hasNoOffset support to actions

5 participants

0