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

Toast: Prevent error being thrown if opened without returnElements setting #572

Merged

Conversation

brentswisher
Copy link
Contributor

This change: (check at least one)

  • Adds a new feature
  • Fixes a bug
  • Improves maintainability
  • Improves documentation
  • Is a release activity

Is this a breaking change? (check one)

  • Yes
  • No

Is the: (complete all)

  • Title of this pull request clear, concise, and indicative of the issue number it addresses, if any?
  • Test suite(s) passing?
  • Code coverage maximal?
  • Changeset added?
  • Component status page up to date?

What does this change address?
Fixes #571 by preventing the default toaster returnElements value from being overwritten to undefined if a toast event is created without specifying it in the pharos-toast-open event.

How does this change work?
If the returnElements coming from the event details is undefined it will be defaulted to an empty array

The toaster element should be defaulting to an empty
array if no returnElements is passed in.
If a custom event was fired without a returnElement, it was
setting the private attribute to undefined. This would
throw an error on toast close because it was not iterable.
@brentswisher brentswisher self-assigned this Jul 28, 2023
@brentswisher brentswisher requested a review from a team as a code owner July 28, 2023 21:21
@brentswisher brentswisher requested review from SMQuazi, drewgingerich and mtorres3 and removed request for a team July 28, 2023 21:21
@changeset-bot
Copy link

changeset-bot bot commented Jul 28, 2023

🦋 Changeset detected

Latest commit: 793c910

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@ithaka/pharos Patch

Not sure what this means? Click here to learn what changesets are.

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

@github-actions
Copy link
Contributor

github-actions bot commented Jul 28, 2023

size-limit report 📦

Path Size
packages/pharos/lib/index.js 47.06 KB (+0.02% 🔺)

Copy link
Member

@daneah daneah left a comment

Choose a reason for hiding this comment

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

Is there a test that could exercise the bug?

@brentswisher
Copy link
Contributor Author

Is there a test that could exercise the bug?

I updated the existing mock to not pass an explicit returnElements in, before it was passing [] So, that is now testing the bug without adding an explicit test.

I changed the test first, saw it failing, made my change, saw it passing so I do think it is covering the changes.

Not opposed to adding one though, if you think that would be better?

@daneah
Copy link
Member

daneah commented Jul 31, 2023

Not opposed to adding one though, if you think that would be better?

That seems sufficient!

@drewgingerich drewgingerich removed their request for review August 1, 2023 17:58
@brentswisher brentswisher merged commit 9f10f87 into develop Aug 2, 2023
8 checks passed
@brentswisher brentswisher deleted the bugfix/prevent-undefined-toast-return-elements branch August 2, 2023 14:44
@github-actions github-actions bot mentioned this pull request Aug 11, 2023
sirrah-tam pushed a commit to sirrah-tam/pharos that referenced this pull request Dec 1, 2023
…tting (ithaka#572)

* test(toast): run tests with default returnElements

The toaster element should be defaulting to an empty
array if no returnElements is passed in.

* fix(toast): ensure returnElements is defined

If a custom event was fired without a returnElement, it was
setting the private attribute to undefined. This would
throw an error on toast close because it was not iterable.

* docs: add @brentswisher as a contributor

* chore(changelog): update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toaster component throws errors when Toast is closed if a returnElements is not provided in the calling event
3 participants