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-7104] - Images create > upload image > reduce space above and below Linode CLI help text #9812

Merged
merged 3 commits into from
Oct 20, 2023
Merged

fix: [M3-7104] - Images create > upload image > reduce space above and below Linode CLI help text #9812

merged 3 commits into from
Oct 20, 2023

Conversation

tyler-akamai
Copy link
Contributor

@tyler-akamai tyler-akamai commented Oct 18, 2023

Description 📝

Changes 🔄

List any change relevant to the reviewer.

  • Decreased spacing above and below the Linode CLI help text

Preview 📷

Before After
Screenshot 2023-10-18 at 10 31 52 AM Screenshot 2023-10-18 at 10 28 38 AM

How to test 🧪

Prerequisites

(How to setup test environment)

  • There are no prerequisites

Reproduction steps

(How to reproduce the issue, if applicable)

Verification steps

(How to verify changes)

As an Author I have considered 🤔

Check all that apply

  • 👀 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

@tyler-akamai tyler-akamai self-assigned this Oct 18, 2023
@tyler-akamai tyler-akamai marked this pull request as ready for review October 18, 2023 14:39
@tyler-akamai tyler-akamai requested a review from a team as a code owner October 18, 2023 14:39
@tyler-akamai tyler-akamai requested review from mjac0bs and jaalah-akamai and removed request for a team October 18, 2023 14:39
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.

Nice catch! When did this get into prod? 😅 Confirmed that the spacing looks normal now, checked all screen sizes. Left a minor suggestion about rewording the changeset.

packages/manager/.changeset/pr-9812-fixed-1697639234272.md Outdated Show resolved Hide resolved
@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Oct 18, 2023
@mjac0bs
Copy link
Contributor

mjac0bs commented Oct 18, 2023

Just had a question after looking at the internal ticket:

Can we clarify with UX whether this ticket is asking for other copy updates to the Upload Image form?

The copy on the proposed change doesn't match prod. I'm a bit concerned by that big block of text in the mock, though. That's in a tooltip now. The mock may just be outdated? Or are we intended to replace the tooltip and warning styling?

Prod:
image

Mock:
Screenshot 2023-10-18 at 8 00 36 AM

Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>
@tyler-akamai
Copy link
Contributor Author

Just had a question after looking at the internal ticket:

Can we clarify with UX whether this ticket is asking for other copy updates to the Upload Image form?

The copy on the proposed change doesn't match prod. I'm a bit concerned by that big block of text in the mock, though. That's in a tooltip now. The mock may just be outdated? Or are we intended to replace the tooltip and warning styling?

Prod: image

Mock: Screenshot 2023-10-18 at 8 00 36 AM

I assumed the mock was outdated since they are still using the Linode logo in the top left, but let me confirm!

@mjac0bs
Copy link
Contributor

mjac0bs commented Oct 18, 2023

still using the Linode logo in the top left,

Oh, hah, yeah, that's very outdated! This ticket was created in Sept of this year, which is what confused me.

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Yay, nice fix!

I had a feeling we would be caught by this one day (the way MUI does number * theme.spacing)

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! Ready for Review labels Oct 18, 2023
@tyler-akamai tyler-akamai merged commit 8dea090 into linode:develop Oct 20, 2023
13 checks passed
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