refactor(alert, notice)!: Renamed properties and updated values #6042
Conversation
BREAKING CHANGE: Renamed `color` properties and updated values. - Removed the property `color`, use `kind` instead. - Updated the accepted values for `kind` to `brand`, `danger`, `info`, `success`, and `warning`.
src/components/interfaces.ts
Outdated
| @@ -16,3 +17,11 @@ export type Width = "auto" | "half" | "full"; | |||
| // used to help track of event payloads to remove at 1.0.0 – see https://github.com/Esri/calcite-components/issues/3781 | |||
| /* Note : should be removed before `1.0 */ | |||
| export type DeprecatedEventPayload = any; | |||
|
|
|||
| export enum KindIcons { | |||
There was a problem hiding this comment.
Is there a better place for this? Previously notice was referencing this from the alert interfaces.ts.
There was a problem hiding this comment.
I thought the enum types were causing issues on the doc site?
This is the correct place for types. constants would go in a resources file in the folder.
There was a problem hiding this comment.
These aren't publicly exposed, so not an issue for doc site. But they are used by more than one component - it seemed weird that previously Notice was referencing this from the Alert resources file - maybe a resources file in the component root is better than this location (although this would be the only thing)?
There was a problem hiding this comment.
Yeah maybe resources is better.
There was a problem hiding this comment.
👍 added a shared resources.ts file to the /component folder root. Will merge after checks run.
| @@ -46,6 +46,7 @@ import { | |||
| LoadableComponent, | |||
| componentLoaded | |||
| } from "../../utils/loadable"; | |||
| import { MenuPlacement } from "../../utils/floating-ui"; | |||
There was a problem hiding this comment.
Note: I updated to use this shared resource instead of explicitly defining in interfaces.ts, since the accepted values are the same. That said, alert doesn't use floating-ui, so I can revert if needed.
|
Merging this - it's approved in Chromatic, not sure why that isn't being reflected for the "UI Tests" check. |
The Chromatic UI Review functionality isn't a required check, we haven't setup any workflows for using it yet. In order to pass the CI you need to accept/deny the individual snapshots by clicking on the UI Tests check's "details" link. Additionally, when the you push more changes to the branch you need to toggle the I created a bash function to automatically toggle the label when you are ready to merge, or when you need to see the results of the snapshots. It requires the GitHub CLI. function ccvsnaps() {
if [[ "$(gh repo view --json name -q ".name")" = "calcite-components" ]]; then
local current_branch
current_branch="$(git symbolic-ref --short HEAD)"
gh pr edit "$current_branch" --remove-label "pr ready for visual snapshots"
gh pr edit "$current_branch" --add-label "pr ready for visual snapshots"
fi
}We also have some known false positives that you can approve if they come up. I'll fix the false positives and add all of this info to CONTRIBUTING when I have a sec. I'll also make Chromatic run when adding a flag to a commit message so the label toggling workflow isn't so annoying lol. Maybe
|
BREAKING CHANGE: Renamed
colorproperties and updated values.color, usekindinstead.kindtobrand,danger,info,success, andwarning.