feat(Seat): add status prop with processing/done states, deprecate selected #4567
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
- Add status enum (default | selected | processing | done) - Mark selected prop as deprecated - Implement background and label styling for new states - Add icon support for processing and done states - Maintain backward compatibility with selected prop Co-Authored-By: Jozef Képesi <[email protected]>
…lected Co-Authored-By: Jozef Képesi <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
|
|
Storybook staging is available at https://kiwicom-orbit-devin-1735834536-seat-status.surge.sh |
![callstackai[bot]](https://avatars.githubusercontent.com/in/1040204?s=60&v=4){kind=small}
There was a problem hiding this comment.
Key Issues
The effectiveStatus calculation lacks validation against allowed status values, risking invalid states and runtime errors. Conditional rendering with effectiveStatus is performed without type narrowing, which could lead to errors if invalid values are used.
| @@ -26,6 +27,7 @@ const Seat = ({ | |||
| description, | |||
| "aria-labelledby": ariaLabelledBy = "", | |||
| }: Props) => { | |||
| const effectiveStatus = status ?? (selected ? "selected" : "default"); | |||
There was a problem hiding this comment.
🐛 Possible Bug
The effectiveStatus calculation doesn't validate the status prop value, which could lead to invalid states being passed down to child components. The status should be validated against allowed values ('default', 'selected', 'processing', 'done') to prevent runtime errors.
Suggested change
| const effectiveStatus = status ?? (selected ? "selected" : "default"); | |
| const validStatuses = ['default', 'selected', 'processing', 'done'] as const; | |
| const effectiveStatus = status && validStatuses.includes(status) ? status : (selected ? 'selected' : 'default'); |
Comment on lines
+75
to
+77
| {effectiveStatus !== "default" && clickable && ( | ||
| <SeatCircle size={size} type={type} status={effectiveStatus} /> | ||
| )} |
There was a problem hiding this comment.
🐛 Possible Bug
The effectiveStatus is used in conditional rendering without type narrowing, which could lead to runtime errors if invalid status values are provided. The status check should be more explicit.
Suggested change
| {effectiveStatus !== "default" && clickable && ( | |
| <SeatCircle size={size} type={type} status={effectiveStatus} /> | |
| )} | |
| {(effectiveStatus === 'selected' || effectiveStatus === 'processing' || effectiveStatus === 'done') && clickable && ( | |
| <SeatCircle size={size} type={type} status={effectiveStatus} /> | |
| )} |
|
Size Change: +1.52 kB (+0.33%) Total Size: 464 kB
ℹ️ View Unchanged
|
Deploying orbit with
|
| Latest commit: |
23eda54
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://499e9ace.orbit.pages.dev |
| Branch Preview URL: | https://devin-1735834536-seat-status.orbit.pages.dev |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adding new
statusprop to the Seat component to replace the deprecatedselectedprop. This change introduces new states for processing and done, while maintaining backward compatibility.Changes
selectedpropstatusprop with values:default | selected | processing | doneprocessinganddonestatesTesting
Link to Devin run: https://app.devin.ai/sessions/4ce5152c9e9a4877b06d0f24ba4a20cf
✨
Description by Callstackai
This PR introduces a new
statusprop to the Seat component, replacing the deprecatedselectedprop. It adds new states for processing and done, along with corresponding styling and icons.Diagrams of code changes
sequenceDiagram participant User participant Seat participant SeatStatus participant Icons Note over Seat: New status prop replaces selected User->>Seat: Interacts with seat alt status="default" Seat->>SeatStatus: Show default state else status="selected" Seat->>SeatStatus: Show selected state Note over Seat: Replaces deprecated selected prop else status="processing" Seat->>SeatStatus: Show processing state Seat->>Icons: Display reload icon Note over Seat: Product light active background else status="done" Seat->>SeatStatus: Show completed state Seat->>Icons: Display check icon Note over Seat: Product light active background end Note over Seat: Status affects visual appearance:<br/>- Background colors<br/>- Label text styles<br/>- Icon visibilityFiles Changed
statusprop and its possible values.statusprop and render appropriate icons for processing and done states.statusprop and adjust rendering logic accordingly.statusprop and its effects on rendering.statusprop for conditional styling.statusprop for conditional styling.statusprop for conditional styling.statusprop instead of the deprecatedselectedprop.SeatStatustype to define possible values for thestatusprop.This PR includes files in programming languages that we currently do not support. We have not reviewed files with the extensions
.md. See list of supported languages.