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

fix: [M3-8773] - Duplicate punctuation on image_upload event message #11148

Conversation

bnussman-akamai
Copy link
Member

@bnussman-akamai bnussman-akamai commented Oct 23, 2024

Description 📝

  • Removes the . we add after the API event message for Image Upload events to fix duplicate punctuation 🔧

Preview 📷

Before After
Screenshot 2024-10-23 at 8 20 43 AM Screenshot 2024-10-23 at 8 31 28 AM

How to test 🧪

  • Begin uploading an image
  • Cancel the upload
  • Delete the Image related to the canceled upload
  • Observe the toast notification 👁️
Screen.Recording.2024-10-23.at.8.35.46.AM.mov

As an Author I have considered 🤔

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@bnussman-akamai bnussman-akamai requested a review from a team as a code owner October 23, 2024 12:38
@bnussman-akamai bnussman-akamai requested review from carrillo-erik and cpathipa and removed request for a team October 23, 2024 12:38
Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

Thanks @bnussman-akamai

The faux bold on your screenshot is indicating the global CSS isn't picked up by the <strong /> tag in safari? Are you seeing this consistently?

Private User Image

@bnussman-akamai
Copy link
Member Author

Yeah, the Safari bolding issue still exists when using <strong /> and I think also <b /> too 🥲 @abailly-akamai

@abailly-akamai
Copy link
Contributor

@bnussman-akamai I'll make a ticket for it if there isn't one, this is not very nice to our safari users

@bnussman-akamai
Copy link
Member Author

That would be great, thank you @abailly-akamai

Copy link

github-actions bot commented Oct 23, 2024

Coverage Report:
Base Coverage: 87.01%
Current Coverage: 87.02%

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Thanks for fixing! Confirmed the rest of the image messages in this file look good as well.

@mjac0bs mjac0bs added the Approved Multiple approvals and ready to merge! label Oct 23, 2024
@bnussman-akamai bnussman-akamai merged commit 93b8065 into linode:develop Oct 23, 2024
23 checks passed
Copy link

cypress bot commented Oct 23, 2024

Cloud Manager E2E    Run #6721

Run Properties:  status check passed Passed #6721  •  git commit 93b80659e0: fix: [M3-8773] - Duplicate punctuation on `image_upload` event message (#11148)
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #6721
Run duration 25m 42s
Commit git commit 93b80659e0: fix: [M3-8773] - Duplicate punctuation on `image_upload` event message (#11148)
Committer Banks Nussman
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 438
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants