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

feat(tabs, tab-nav, tab-title, tab): add component tokens #10532

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

Conversation

jcfranco
Copy link
Member

@jcfranco jcfranco commented Oct 12, 2024

Related Issue: #7180, #10497

Summary

Adds the following component tokens (CSS props):

--calcite-tab-accent-color-hover – Specifies the component's accent color when hovered.
--calcite-tab-accent-color-selected – Specifies the component's accent color when selected.
--calcite-tab-background-color – Specifies the component's background color.
--calcite-tab-background-color-hover – Specifies the component's background color when hovered.
--calcite-tab-content-space-y – Specifies the block padding of the component's content.
--calcite-tab-icon-color – Specifies the component's icon color.
--calcite-tab-icon-end-color – Specifies the component's `iconEnd` color. Fallback to `--calcite-tab-icon-color` or current color.
--calcite-tab-icon-start-color – Specifies the component's `iconStart` color. Fallback to `--calcite-tab-icon-color` or current color.
--calcite-tab-text-color – Specifies the component's text color.
--calcite-tab-text-color-selected – Specifies the component's text color when selected.
--calcite-tabs-background-color – The background color of the component.
--calcite-tabs-border-color – The border color of the component.

Deprecates the following CSS props:

--calcite-tab-content-block-padding

Notes

alisonailea and others added 27 commits September 16, 2024 12:08
:host
.class-selectors
:host([prop]) selectors
@includes
@Keyframes
simplify background color and border across appearance and kind
…openPR/7180-button

# Conflicts:
#	packages/calcite-components/src/components/button/button.e2e.ts
* icon-color
* icon-start-color
* icon-end-color
* x-button
* drop unused anchor rule
* border
@jcfranco jcfranco added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Oct 12, 2024
@jcfranco jcfranco changed the title WIP (button): component tokens feat(tabs, tab-nav, tab-title, tab): add component tokens Oct 12, 2024
@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 15, 2024
@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 18, 2024
@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 21, 2024
@jcfranco jcfranco marked this pull request as ready for review October 21, 2024 17:04
@jcfranco jcfranco requested a review from benelan as a code owner October 21, 2024 17:04
@jcfranco jcfranco requested review from alisonailea, eriklharper, macandcheese and Elijbet and removed request for benelan October 21, 2024 17:06
@@ -46,36 +50,6 @@
<div class="child">large</div>
</div>

<!-- simple tab-nav -->
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed since this is not a supported configuration.

Copy link
Contributor

@alisonailea alisonailea left a comment

Choose a reason for hiding this comment

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

Looks good Franco. Given our in-person conversation, I think most of the -hover -press tokens can be removed as these are slotted components. Also can the x-button be pulled into a function/mixin for reusability as well as the "accent-color" styles which I think should be shared across the app but I'd defer to @SkyeSeitz and @ashetland for that.

I was having a hard time explaining the change requests in comments so I made a draft PR #10579

@@ -19,7 +19,7 @@
.content {
@apply text-n2h py-1;
}
.close-button {
.x-button {
Copy link
Contributor

Choose a reason for hiding this comment

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

should these x-button styles be moved to a shared mixin?

@@ -3,7 +3,7 @@
*
* These properties can be overridden using the component's tag as selector.
*
* @prop --calcite-tab-content-block-padding: Specifies the block padding of the component's content in the `default` slot.
* @prop --calcite-tab-content-block-padding: [Deprecated] Use `--calcite-tab-content-space-y` instead. Specifies the block padding of the component's content in the `default` slot.
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's documented in "tabs" but I think we should add --calcite-tab-content-space-y docs here as well since it's used in the component.

@@ -3,7 +3,7 @@
*
* These properties can be overridden using the component's tag as selector.
*
* @prop --calcite-tab-content-block-padding: Specifies the block padding of the component's content in the `default` slot.
* @prop --calcite-tab-content-block-padding: [Deprecated] Use `--calcite-tab-content-space-y` instead. Specifies the block padding of the component's content in the `default` slot.
*/

:host([selected]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you replace lines 9 - 24 with the following to clean up styles

:host {
  @apply hidden h-full w-full;
}

:host([selected]) {
  @apply block h-full w-full overflow-auto;

  section,
  .container {
    @apply block;
  }
}

Copy link
Contributor

github-actions bot commented Nov 1, 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 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing. Stale Issues or pull requests that have not had recent activity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants