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 icon margin within small buttons #5387

Merged
merged 4 commits into from
Oct 18, 2024
Merged

Conversation

mcslayer
Copy link
Contributor

@mcslayer mcslayer commented Oct 13, 2024

Done

Based on my understanding, the logic is as follows:
When no icon exists, the space is applied symmetrically to both the left and right sides: [x<span>x].
When an icon is added, the space is adjusted accordingly: either [x/2<icon>x/2<span>x] or [x<span>x/2<icon>x/2].

I have fixed this and included additional examples for the button/icon section for clarity.

#5386

QA

Check if PR is ready for release

If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:

  • PR should have one of the following labels to automatically categorise it in release notes:
    • Feature 🎁, Breaking Change 💣, Bug 🐛, Documentation 📝, Maintenance 🔨.
  • Vanilla version in package.json should be updated relative to the most recent release, following semver convention:
    • if CSS class names are not changed it can be bugfix release (x.x.X)
    • if CSS class names are changed/added/removed it should be minor version (x.X.0)
    • see the wiki for more details
  • Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.
image

@webteam-app
Copy link

mcslayer is not a collaborator of the repo

scss/_base_button.scss Outdated Show resolved Hide resolved
@bartaz
Copy link
Member

bartaz commented Oct 14, 2024

Mykola Ptushchuk added 2 commits October 14, 2024 13:40
@mcslayer
Copy link
Contributor Author

@bartaz Hi! I’ve finished working on the code, fixed the issues, and reviewed everything we discussed. The code is now ready for further review or implementation. I’d appreciate it if you could take a look so we can move forward.

package.json Outdated Show resolved Hide resolved
Co-authored-by: Bartek Szopka <83575+bartaz@users.noreply.github.com>
@bartaz bartaz changed the title Fix margin for button with icon Fix icon margin within small buttons Oct 17, 2024
@dgtlntv
Copy link
Member

dgtlntv commented Oct 18, 2024

LGTM Design +1

Copy link
Member

@bartaz bartaz left a comment

Choose a reason for hiding this comment

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

Thanks @mcslayer for the fix and responsiveness. That's really helpful.

@bartaz bartaz merged commit 8e3f6f8 into canonical:main Oct 18, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants