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

Remove modern syntax in NotificationInline #1435

Merged
merged 3 commits into from
Feb 15, 2022
Merged

Conversation

connor-baer
Copy link
Member

@connor-baer connor-baer commented Feb 15, 2022

Purpose

Modern JavaScript syntax isn't transpiled in Circuit UI and can lead to build issues in apps that consume Circuit UI.

Approach and changes

  • Removed optional chaining in the NotificationInline component
  • Bonus: Automatically clean up build artifacts before running yarn bootstrap

Definition of done

  • Development completed
  • Reviewers assigned
  • Unit and integration tests
  • Meets minimum browser support
  • Meets accessibility requirements

@changeset-bot
Copy link

changeset-bot bot commented Feb 15, 2022

🦋 Changeset detected

Latest commit: 0a2537a

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

This PR includes changesets to release 1 package
Name Type
@sumup/circuit-ui 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

@connor-baer connor-baer marked this pull request as ready for review February 15, 2022 13:26
@connor-baer connor-baer requested a review from a team as a code owner February 15, 2022 13:26
@connor-baer connor-baer requested review from robinmetral and removed request for a team February 15, 2022 13:26
@vercel
Copy link

vercel bot commented Feb 15, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sumup/oss-circuit-ui/8AfMcoL12ptAFnY5jEKLDkYL3NYy
✅ Preview: https://oss-circuit-ui-git-bugfix-optional-chaining-sumup.vercel.app

@connor-baer connor-baer added the 🐞 bug Something isn't working as it should label Feb 15, 2022
@connor-baer connor-baer changed the title Remove optional chaining in NotificationInline Remove modern syntax in NotificationInline Feb 15, 2022
@codecov
Copy link

codecov bot commented Feb 15, 2022

Codecov Report

Merging #1435 (0a2537a) into main (1f5c5fa) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1435      +/-   ##
==========================================
+ Coverage   92.31%   92.33%   +0.02%     
==========================================
  Files         193      193              
  Lines        3889     3889              
  Branches     1213     1213              
==========================================
+ Hits         3590     3591       +1     
+ Misses        280      279       -1     
  Partials       19       19              
Impacted Files Coverage Δ
...mponents/NotificationInline/NotificationInline.tsx 92.72% <100.00%> (ø)
...mponents/NotificationBanner/NotificationBanner.tsx 83.60% <0.00%> (+1.63%) ⬆️

@connor-baer connor-baer added the 🚢 ready to merge Automatically merge the PR once all requirements are met label Feb 15, 2022
@kodiakhq kodiakhq bot merged commit 55e7878 into main Feb 15, 2022
@kodiakhq kodiakhq bot deleted the bugfix/optional-chaining branch February 15, 2022 13:39
@github-actions github-actions bot mentioned this pull request Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working as it should 🗂 circuit-ui 🚢 ready to merge Automatically merge the PR once all requirements are met
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants