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/3055/add padding between checkbox list and label #3077

Merged

Conversation

balibirchlee
Copy link
Contributor

What is the context of this PR?

Aligns padding of checkbox label with other text - rather than 'adding' padding, I actually removed the padding css that was specifically reducing the padding of checkbox labels.

The VR tests are passing locally but failing here - I think a github issue not a code issue.

Fixes: #3055

How to review this PR

You can see the difference on the checkboxes pages.

Checklist

This needs to be completed by the person raising the PR.

  • I have selected the correct Assignee
  • I have linked the correct Issue

Copy link

netlify bot commented Mar 8, 2024

Deploy Preview for ons-design-system-preview ready!

Name Link
🔨 Latest commit 0479166
🔍 Latest deploy log https://app.netlify.com/sites/ons-design-system-preview/deploys/660fef9e3a42740007daa6ac
😎 Deploy Preview https://deploy-preview-3077--ons-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@alessioventuriniAND alessioventuriniAND added the Bug Something isn't working label Mar 11, 2024
Copy link
Contributor

@rmccar rmccar left a comment

Choose a reason for hiding this comment

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

I think the margin after the "or" is now missing on mutually exclusive options
Screenshot 2024-03-11 at 15 42 14

Copy link
Contributor

@alessioventuriniAND alessioventuriniAND left a comment

Choose a reason for hiding this comment

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

Same as Richard's comment. Once you fix that I am happy to approve too :)

@adi-unni
Copy link
Contributor

adi-unni commented Mar 18, 2024

Happy to approve code-wise but might need to get Dina's approval too as there are a lot of changes outside of just the issue.

Copy link
Contributor

@rmccar rmccar left a comment

Choose a reason for hiding this comment

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

Like Adi mentioned this seems to have had some potential unintended changes. Needs to be checked with Dina/Joe. To me the "Select all that apply" looks a bit far away now but see what she says.

@balibirchlee
Copy link
Contributor Author

@adi-unni @rmccar

Dina confirmed she's happy with the all the visual changes!

@alessioventuriniAND alessioventuriniAND merged commit 1afcb6d into main Apr 8, 2024
9 checks passed
@alessioventuriniAND alessioventuriniAND deleted the fix/3055/add-padding-between-checkbox-list-and-label branch April 8, 2024 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add padding between checkbox list and label
4 participants