Skip to content

Conversation

tylersticka
Copy link
Member

Overview

Addresses an issue where icons were inconsistently sized if included as the primary button content, as one might expect for an icon-only button.

Previously, button icon sizes were controlled via font-size on the icon container, c-button__extra. When an icon was included as the primary content, c-button__content, there was no such font-size declaration, and so the icon would appear smaller by comparison.

I considered adding some utility classes and/or icon modifiers to solve this problem, but I realized when futzing with the component that there were other potential issues with relying on font-size on a containing element. For example, it's impossible to reliably negate the gap between the __extra and __content elements since the font-size for the icon increases the computed size of that gap to a spot between two of our modular scale steps.

Instead, I added an optional --icon-size property to the c-icon component. This allows us to set the desired size of the icon on c-button, which cascades to all elements below. It also means the computed impact of the icon's size is not effective until the icon itself, so any margins or other relative values on its container will be more consistent with adjacent elements.

Along the way, I noticed that the c-button__extra styles were being unnecessarily output a second time within the context of WordPress buttons. I separated those styles into their own mixins to prevent this.

(This PR originally included other changes to c-button to streamline its template code, but I realized partway in that the actual solution to the issue was a smaller subset of that, which I've separated into this PR.)

Screenshots

Before

Screen Shot 2021-11-16 at 2 33 54 PM

After

Screen Shot 2021-11-16 at 2 40 35 PM

Testing

On the deploy preview…

  1. Review Button component for regressions
  2. Review Icon component for regressions
  3. Review Article Listing prototype search form for regressions

@changeset-bot
Copy link

changeset-bot bot commented Nov 16, 2021

🦋 Changeset detected

Latest commit: 1f0d855

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cloudfour/patterns Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify
Copy link

netlify bot commented Nov 16, 2021

✔️ Deploy Preview for cloudfour-patterns ready!

🔨 Explore the source changes: 1f0d855

🔍 Inspect the deploy log: https://app.netlify.com/sites/cloudfour-patterns/deploys/6194342f650c5600085e1e48

😎 Browse the preview: https://deploy-preview-1586--cloudfour-patterns.netlify.app

@tylersticka tylersticka marked this pull request as ready for review November 16, 2021 22:49
@tylersticka tylersticka requested review from a team November 16, 2021 22:50
Copy link
Member

@spaceninja spaceninja left a comment

Choose a reason for hiding this comment

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

:shipit:

@tylersticka tylersticka merged commit 9819243 into v-next Nov 17, 2021
@tylersticka tylersticka deleted the feature/icon-size branch November 17, 2021 00:54
@github-actions github-actions bot mentioned this pull request Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability for search input's icon to use an increased font size
2 participants