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

Add new token for disabled-text #195 #196

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blackfalcon
Copy link
Member

@blackfalcon blackfalcon commented Nov 27, 2024

Alaska Airlines Pull Request

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Resolves: #195

Summary:

This commit creates a new semantic token specifically to be used in UIs where the content of an interaction is to appear as disabled to the end user. This new token may be used in cases where there is unavailable features in the UI that are not specifically interactive elements.

On branch dsande/disabledText/195
Changes to be committed:
modified: src/color/text.json

Type of change:

Please delete options that are not relevant.

  • New capability
  • Revision of an existing capability
  • Infrastructure change (automation, etc.)
  • Other (please elaborate)

Checklist:

  • My update follows the CONTRIBUTING guidelines of this project
  • I have performed a self-review of my own update

By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.

Thank you for your submission!

-- Auro Design System Team

Summary by Sourcery

Add a new semantic token for disabled text color to enhance UI design by providing a distinct color for non-interactive elements. Update the existing disabled UI text color to improve visual consistency.

New Features:

  • Introduce a new semantic token for disabled text color to be used in UI elements that are not interactive.

Enhancements:

  • Update the disabled UI text color on light backgrounds to a new shade of gray for better consistency.

This commit creates a new semantic token specifically to be used in UIs
where the content of an interaction is to appear as disabled to the end
user. This new token may be used in cases where there is unavailable
features in the UI that are not specifically interactive elements.

On branch dsande/disabledText/195
Changes to be committed:
modified:   src/color/text.json
@blackfalcon blackfalcon self-assigned this Nov 27, 2024
@blackfalcon blackfalcon requested a review from a team as a code owner November 27, 2024 20:22
Copy link

sourcery-ai bot commented Nov 27, 2024

Reviewer's Guide by Sourcery

This PR introduces a new semantic token for disabled text and updates existing disabled UI text colors. The implementation adds a new 'disabled' token group in text.json and modifies the existing UI disabled text color value to maintain consistency.

Entity Relationship Diagram for text.json changes

erDiagram
    TEXT_JSON {
        string default_value
        string public
        string default
        string usage
        string wcag
        boolean deprecated
    }
    TEXT_JSON ||--o{ DISABLED : contains
    DISABLED {
        string value
        boolean public
        boolean default
        string usage
        string wcag
        boolean deprecated
    }
    TEXT_JSON ||--o{ UI : contains
    UI {
        string value
        boolean public
        boolean default
        string usage
    }
Loading

File-Level Changes

Change Details Files
Added new semantic token for disabled text states
  • Created new 'disabled' token group with default and inverse variants
  • Set default disabled text color to gray.500
  • Set inverse disabled text color to gray.600
  • Added usage descriptions for both variants
  • Marked both variants as public and not deprecated
src/color/text.json
Updated existing UI disabled text color
  • Changed UI disabled text default color from gray.400 to gray.500 for consistency with new disabled text token
src/color/text.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @blackfalcon - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Please specify WCAG contrast ratios instead of marking as 'n/a' since this involves text visibility
  • Consider documenting the relationship between 'text.disabled' and 'text.ui.disabled' tokens since they share the same color value
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +166 to +168
"disabled": {
"default": {
"value": "{color.base.gray.500.value}",
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider consolidating the duplicate disabled text color definitions

There are now two tokens for disabled text that use the same value - the new top-level 'disabled.default' and 'ui.disabled.default'. Consider either making one reference the other or consolidating them if there isn't a strong semantic difference between UI and non-UI disabled states.

      "disabled": {
        "default": {
          "value": "{ui.disabled.default.value}",

@blackfalcon blackfalcon linked an issue Nov 27, 2024 that may be closed by this pull request
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.

a11y: Add token to support disabled text
1 participant