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.

Sign up for GitHub

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

Jump to bottom

feat(button): add component tokens #10358

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

feat(button): add component tokens #10358

wants to merge 3 commits into from

Conversation

alisonailea
Copy link
Contributor

@alisonailea alisonailea commented Sep 19, 2024 •
edited
Loading

Related Issue: #7180

Summary

Calcite Button

--calcite-button-background-color: Specifies the component's background color when appearance="solid" or appearance="outline-fill".
--calcite-button-border-color: Specifies the component's border color when it has appearance="outline" or appearance="outline-fill".
--calcite-button-corner-radius: Specifies the component's corner radius.
--calcite-button-text-color: Specifies the component's text color.

@alisonailea alisonailea self-assigned this Sep 19, 2024
@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Sep 19, 2024
@alisonailea alisonailea mentioned this pull request Sep 25, 2024
Open
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 3, 2024

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added Stale Issues or pull requests that have not had recent activity. and removed Stale Issues or pull requests that have not had recent activity. labels Oct 3, 2024
@alisonailea alisonailea added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Oct 7, 2024
@jcfranco
Copy link
Member

For consistency and according to the wiki, button needs stateful tokens.

@alisonailea
Copy link
Contributor Author

For consistency and according to the wiki, button needs stateful tokens.

That's not the case. State tokens are only for sub-components

@jcfranco jcfranco mentioned this pull request Oct 15, 2024
Open
@jcfranco
Copy link
Member

Synced up w/ @alisonailea and got clarification. The guidance doc has been updated, so we are good to proceed.

jcfranco
jcfranco requested changes Oct 18, 2024
View reviewed changes
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

This is looking great, @alisonailea! 😎

packages/calcite-components/src/components/button/button.scss Show resolved Hide resolved
packages/calcite-components/src/components/button/button.scss Show resolved Hide resolved
packages/calcite-components/src/components/button/button.scss Outdated Show resolved Hide resolved
packages/calcite-components/src/components/button/button.e2e.ts Outdated Show resolved Hide resolved
@alisonailea alisonailea requested a review from jcfranco October 22, 2024 02:38
@jcfranco jcfranco changed the title feat(button): add component tokens feat(button, fab): add component tokens Oct 22, 2024
jcfranco
jcfranco approved these changes Oct 22, 2024
View reviewed changes
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Nice! This should be good to merge once the remaining comments are addressed. Thanks, @alisonailea! ✨💪✨

packages/calcite-components/src/components/fab/fab.scss Outdated Show resolved Hide resolved
packages/calcite-components/src/custom-theme/button.ts Outdated Show resolved Hide resolved
@alisonailea alisonailea mentioned this pull request Oct 25, 2024
Merged
@jcfranco jcfranco added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Oct 30, 2024
@jcfranco jcfranco mentioned this pull request Oct 30, 2024
Closed
6 tasks
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 8, 2024

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Nov 8, 2024
@alisonailea
Copy link
Contributor Author

Waiting on #10829 #10828 #10827

@alisonailea alisonailea removed the Stale Issues or pull requests that have not had recent activity. label Nov 21, 2024
@github-actions GitHub Actions
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Nov 29, 2024
@alisonailea alisonailea marked this pull request as draft December 30, 2024 18:36
@github-actions github-actions bot removed the Stale Issues or pull requests that have not had recent activity. label Dec 31, 2024
@driskull
Copy link
Member

driskull commented Jan 6, 2025

@alisonailea for button, can we also add a token for setting the border width on each side? This would provide the ability to remove a border on one side of a button in order place buttons side by side.

@alisonailea alisonailea changed the title feat(button, fab): add component tokens feat(button): add component tokens Jan 10, 2025
@alisonailea alisonailea marked this pull request as ready for review January 10, 2025 08:14
@alisonailea
Copy link
Contributor Author

@alisonailea for button, can we also add a token for setting the border width on each side? This would provide the ability to remove a border on one side of a button in order place buttons side by side.

@driskull you can now set the --calcite-button-border-width when the button props set a border

macandcheese
macandcheese reviewed Jan 10, 2025
View reviewed changes
packages/calcite-components/src/components/button/button.scss
* @prop --calcite-button-loader-color: Specifies the component's loader color.
* @prop --calcite-button-shadow-color: Specifies the component's box-shadow color.
* @prop --calcite-button-text-color: Specifies the component's text color.
* @prop --calcite-button-border-width: Specifies the component's border width.
Copy link
Contributor

Choose a reason for hiding this comment

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

For @driskull case - would this need to be inline-border-start-width / inline-border-end-width?

I'd almost suggest marking those as internal as well - we may be able to support the "button group" use case in a different way down the line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Elijbet Elijbet Elijbet left review comments

@macandcheese macandcheese macandcheese left review comments

@jcfranco jcfranco jcfranco approved these changes

@eriklharper eriklharper Awaiting requested review from eriklharper

@driskull driskull Awaiting requested review from driskull

@benelan benelan Awaiting requested review from benelan

@anveshmekala anveshmekala Awaiting requested review from anveshmekala

@josercarcamo josercarcamo Awaiting requested review from josercarcamo

@aPreciado88 aPreciado88 Awaiting requested review from aPreciado88

Assignees

@alisonailea alisonailea

Labels
enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@alisonailea @jcfranco @driskull @macandcheese @Elijbet