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: #1045 chevron icon in picker got squeezed #1056

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

jianliao
Copy link
Contributor

@jianliao jianliao commented Oct 15, 2020

Description

Since the new chevron ui icon increased its size to 10x10. It got squeezed in some scenarios.

Fixed #1045

How and where has this been tested?

  • How this was tested:
  • Browser(s) and OS(s) this was tested with:

Screenshots

Before:
image

After:
localhost_3000_docs_picker html (1)

To-do list

  • If my change impacts other components, I have tested to make sure they don't break.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • This pull request is ready to merge.

@adobe-spectrum-bot
Copy link
Collaborator

Spectrum CSS documentation build successfully! 🎉

View the documentation

@jianliao jianliao requested review from GarthDB and lazd October 15, 2020 20:55
@adobe-spectrum-bot
Copy link
Collaborator

VRT successfully! 🎊

View the VRT result

@lazd
Copy link
Member

lazd commented Oct 16, 2020

Interesting, so it has 2px of extra whitespace or something?

What about the fact that the class and the ID don't match?

          <svg class="spectrum-Icon spectrum-UIIcon-ChevronDown100 spectrum-Picker-icon" focusable="false" aria-hidden="true">
            <use xlink:href="#spectrum-css-icon-Chevron100" />
          </svg>

We have ChevronDown100 and Chevron100, what's up with that?

@jianliao
Copy link
Contributor Author

If I understand correctly, we have icon css class spectrum-UIIcon-ChevronDown100. But we don't have an SVG icon id named spectrum-UIIcon-ChevronDown100.

@jianliao
Copy link
Contributor Author

The chevron icon size changed from 10x6 to 10x10.

@lazd
Copy link
Member

lazd commented Oct 16, 2020

@jianliao then let's rename the class to match, classes and icon names should be identical, I don't know if they've ever been different before.

@lazd lazd merged commit 12fb3cc into spectrum-tokens-18 Oct 20, 2020
@GarthDB GarthDB deleted the fix-picker-trigger-icon branch September 30, 2021 20:41
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.

3 participants