Skip to content

chore(popover): fix dismissible spelling #3689

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
Merged
merged 1 commit into from
May 5, 2025
Merged

chore(popover): fix dismissible spelling #3689

merged 1 commit into from
May 5, 2025

Conversation

marissahuysentruyt
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt commented May 2, 2025 •
edited
Loading

Description

This PR corrects the spelling of the dismissible arg in the dialog-style popover. In ##2860, the arg spelling was updated, and we must have missed correcting the spelling in this popover story.

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

  • Pull down the branch to run locally or use the deploy preview.
  • Spot check a few pages to make sure things run and render as expected.
  • Visit the popover docs page. Verify the "dialog-style content" story renders with no close button (which is expected 👍 ). There should be no regressions.
  • While running the project locally, edit the DialogStyle popover story so that the dialog shows the close button. Set isDismissible: true. (you'll revert this change after validating!)
  • On the docs page, the close button in the popover should render appropriately. With the misspelled arg, this was not working properly.
Screenshot 2025-05-05 at 11 16 26 AM

Regression testing

Validate:

  1. The documentation pages for at least two other components are still loading, including:
  • The pages render correctly, are accessible, and are responsive.
  1. If components have been modified, VRTs have been run on this branch:
  • VRTs have been run and looked at.
  • Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • ✨ This pull request is ready to merge. ✨

@marissahuysentruyt marissahuysentruyt changed the base branch from main to spectrum-two May 2, 2025 17:33
@marissahuysentruyt marissahuysentruyt changed the base branch from spectrum-two to marissahuysentruyt/docs-add-migrated-tags May 2, 2025 17:34
@marissahuysentruyt marissahuysentruyt self-assigned this May 2, 2025
@marissahuysentruyt marissahuysentruyt marked this pull request as ready for review May 5, 2025 15:00
@marissahuysentruyt marissahuysentruyt added size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. S2 Spectrum 2 labels May 5, 2025
@marissahuysentruyt marissahuysentruyt changed the base branch from marissahuysentruyt/docs-add-migrated-tags to spectrum-two May 5, 2025 15:01
@marissahuysentruyt marissahuysentruyt added ready-for-review skip_vrt Add to a PR to skip running VRT (but still pass the action) labels May 5, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 5, 2025 •
edited
Loading

File metrics

Summary

Total size: 1.38 MB*

🎉 No changes detected in any packages

* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 5, 2025 •
edited
Loading

🚀 Deployed on https://pr-3689--spectrum-css.netlify.app

castastrophe
castastrophe approved these changes May 5, 2025
View reviewed changes
Copy link
Collaborator

@castastrophe castastrophe left a comment

Choose a reason for hiding this comment

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

Woo! Great catch and thanks for sneaking in those migrated tags too! 😉

components/popover/stories/popover.stories.js
@@ -246,7 +247,7 @@ DialogStyle.args = {
(passthroughs, context) => Dialog({
showModal: false,
size: "s",
isDismissable: false,
isDismissible: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

@marissahuysentruyt
docs: fix spelling of dismissible
ee7a692 
because the spelling of the "dismissible" arg changed in the dialog
component, the spelling needed to be updated in popover as well.
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/fix-dismissible-spelling branch from a80115e to ee7a692 Compare May 5, 2025 16:08
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented May 5, 2025

⚠️ No Changeset found

Latest commit: ee7a692

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@marissahuysentruyt marissahuysentruyt merged commit f00a83b into spectrum-two May 5, 2025
12 checks passed
@marissahuysentruyt marissahuysentruyt deleted the marissahuysentruyt/fix-dismissible-spelling branch May 5, 2025 16:23
This was referenced May 16, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@castastrophe castastrophe castastrophe approved these changes

Assignees

@marissahuysentruyt marissahuysentruyt

Labels
ready-for-review S2 Spectrum 2 size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. skip_vrt Add to a PR to skip running VRT (but still pass the action)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@marissahuysentruyt @castastrophe