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

Prevent expanding/collapsing a notification when clicking the action button #48074

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

gzdunek
Copy link
Contributor

@gzdunek gzdunek commented Oct 29, 2024

Clicking on the action button should not expand/collapse notifications:

notification.mov

@gzdunek gzdunek added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v17 labels Oct 29, 2024
@gzdunek gzdunek requested review from ravicious and bl-nero October 29, 2024 15:31
@github-actions github-actions bot requested review from rudream and ryanclark October 29, 2024 15:31
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-48074.d3pp5qlev8mo18.amplifyapp.com

content: action.content,
onClick: event => {
// Prevents toggling the isExpanded flag.
event.stopPropagation();
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way we can add a test for this? It's a rather niche interaction that can easily regress in the future. OTOH, as isExpanded merely controls the styles, I don't think the JSDOM-based tests are going to be able to detect the difference.

Copy link
Contributor Author

@gzdunek gzdunek Oct 30, 2024

Choose a reason for hiding this comment

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

Yeah, we can't detect the text overflow. Instead, I added a test that checks if the style wasn't modified by the click. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

That should be good enough for now! I didn't realize that toHaveStyleRule works with CSS in general, I've always assumed it only works when the style is set directly on a node.

@gzdunek gzdunek added this pull request to the merge queue Oct 30, 2024
Merged via the queue into master with commit af117a3 Oct 30, 2024
40 checks passed
@gzdunek gzdunek deleted the gzdunek/stop-propagation-action-button branch October 30, 2024 16:00
@public-teleport-github-review-bot

@gzdunek See the table below for backport results.

Branch Result
branch/v17 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry size/sm ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants