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-7116] Fix visual regression with Image Select option #9672

Merged
merged 5 commits into from
Sep 13, 2023

Conversation

abailly-akamai
Copy link
Contributor

@abailly-akamai abailly-akamai commented Sep 13, 2023

Description 📝

This PR fixes a small regression with the image select options introduced by #9636

The defect is not in production yet, so the current fix should be evaluated as re-establishing parity with production in our development branch. Therefore we don't need a changeset.

Note: the PR also cleans up import/exports and React.FC props in files touched by the fix.

Preview 📷

Before After
Screenshot 2023-09-13 at 13 26 27 Screenshot 2023-09-13 at 13 23 09

How to test 🧪

  1. Navigate to linode/create and validate that the styling of the image creation dropdown options is identical to production

@@ -27,6 +28,9 @@ const useStyles = makeStyles((theme: Theme) => ({
color: 'white',
},
root: {
'& *': {
lineHeight: '1.2em',
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the relevant change - in this case it only applies to the image select options, but we should keep an eye on other selects. Could not find similar regressions anywhere else during testing.

@abailly-akamai abailly-akamai self-assigned this Sep 13, 2023
@abailly-akamai abailly-akamai marked this pull request as ready for review September 13, 2023 17:28
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.

Thanks for the quick fix! 🔨 Other selects I checked looked good and great cleanup as well 🧹

Copy link
Contributor

@jaalah-akamai jaalah-akamai left a comment

Choose a reason for hiding this comment

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

Confirmed. 🚀

@jaalah-akamai jaalah-akamai added the Approved Multiple approvals and ready to merge! label Sep 13, 2023
@abailly-akamai abailly-akamai merged commit cc2481a into linode:develop Sep 13, 2023
11 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.

3 participants