Skip to content

Conversation

@HerrTopi
Copy link
Contributor

TEST_PLAN: compare with design documentation

INSTUI-4547

@HerrTopi HerrTopi requested review from balzss, Copilot and joyenjoyer May 15, 2025 11:13
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new text color variant "ai-highlight" to the UI Text component and updates related types and styles.

  • Added new theme colors: aiColor and aiBackgroundColor.
  • Updated style definitions and prop types to support the new "ai-highlight" variant.
  • Adjusted documentation examples and shared type definitions for color properties.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/ui-text/src/Text/theme.ts Added aiColor and aiBackgroundColor to the theme
packages/ui-text/src/Text/styles.ts Updated style definitions to include the ai-highlight variant
packages/ui-text/src/Text/props.ts Extended supported color types with ai-highlight
packages/ui-text/src/Text/README.md Updated documentation to showcase the new ai-highlight example
packages/shared-types/src/ComponentThemeVariables.ts Changed color properties types from Colors references to string

successColor: colors?.contrasts?.green5782,
alertColor: colors?.contrasts?.blue5782,
warningColor: colors?.contrasts?.orange5782,
aiColor: colors.contrasts?.violet5790,
Copy link

Copilot AI May 15, 2025

Choose a reason for hiding this comment

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

For consistency with the other color properties, consider using the safe navigation operator on 'colors' (i.e. colors?.contrasts?.violet5790) to prevent potential runtime errors if colors is null or undefined.

Suggested change
aiColor: colors.contrasts?.violet5790,
aiColor: colors?.contrasts?.violet5790,

Copilot uses AI. Check for mistakes.
successColor: Colors['contrasts']['green4570']
alertColor: Colors['contrasts']['blue4570']
warningColor: Colors['contrasts']['orange5782']
primaryInverseColor: string
Copy link

Copilot AI May 15, 2025

Choose a reason for hiding this comment

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

Changing the color properties from specific Colors references to a generic string type could reduce type safety. Consider maintaining the original color type definitions or adding validation to ensure consistency in the color values.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

github-actions bot commented May 15, 2025

PR Preview Action v1.6.1

🚀 View preview at
https://instructure.design/pr-preview/pr-1972/

Built to branch gh-pages at 2025-05-16 10:53 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@HerrTopi HerrTopi merged commit b31d917 into master May 16, 2025
7 checks passed
@HerrTopi HerrTopi deleted the ui-text branch May 16, 2025 10:50
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.

4 participants