chore: improve supported domain initial workflow #8054
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
nadr0
changed the title
|
Sorry not at my laptop, tiny CSS feedback:
|
…KittyCAD/modeling-app into nadro/adhoc/environment-quality-of-life
…KittyCAD/modeling-app into nadro/adhoc/environment-quality-of-life
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
src/routes/AdvancedSignInOptions.tsx
Outdated
| className={`text-xs px-1 ${ | ||
| checked ? 'text-white' : 'text-white' | ||
| }`} |
There was a problem hiding this comment.
The conditional class assignment here is redundant since both branches apply text-white. This appears to be unintentional, as the text color should typically differ between selected and unselected states. Consider updating to something like:
className={`text-xs px-1 ${checked ? 'text-white' : 'text-chalkboard-70'}`}This would provide better visual distinction between the selected and unselected radio options.
| className={`text-xs px-1 ${ | |
| checked ? 'text-white' : 'text-white' | |
| }`} | |
| className={`text-xs px-1 ${ | |
| checked ? 'text-white' : 'text-chalkboard-70' | |
| }`} |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| Advanced options | ||
| <CustomIcon | ||
| name="caretDown" | ||
| className={`w-4 h-4 ${!showCustomInput ? '' : 'ui-open:rotate-180'} `} |
There was a problem hiding this comment.
The ui-open:rotate-180 class is designed for Headless UI disclosure components where the ui-open: modifier is automatically applied. Since this is a standard button toggle using React state, the rotation should be directly tied to the state variable:
className={`w-4 h-4 ${showCustomInput ? 'rotate-180' : ''}`}This will ensure the caret rotates correctly when the advanced options are toggled.
| className={`w-4 h-4 ${!showCustomInput ? '' : 'ui-open:rotate-180'} `} | |
| className={`w-4 h-4 ${showCustomInput ? 'rotate-180' : ''} `} |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| {showCustomInput && ( | ||
| <Combobox.Input | ||
| className="gap-1 rounded h-6 border px-2 flex items-center dark:hover:bg-chalkboard-80 text-xs text-chalkboard-70 dark:text-chalkboard-30 dark:bg-chalkboard-90" | ||
| placeholder="auto" | ||
| onChange={(event) => | ||
| setSelectedEnvironment(event.target.value) | ||
| } | ||
| /> | ||
| )} |
There was a problem hiding this comment.
UI State Synchronization Issue
There's a potential state synchronization issue between the radio group selection and the custom domain input. When a user:
- Selects a predefined domain via the radio group
- Opens advanced options (the custom input will show this selected value)
- Closes advanced options without modifying the input
The application maintains the selectedEnvironment value but the UI no longer clearly indicates which input method is active. This creates a disconnected state where the selected value exists but isn't visibly represented in the UI.
Consider one of these approaches:
- Maintain separate state for radio selection vs. custom input, with logic to reconcile them
- Add synchronization logic to ensure the radio group reflects the current
selectedEnvironmentwhen toggling advanced options - Implement a clear visual indication of which input method is currently controlling the domain value
This would improve the user experience by maintaining a consistent relationship between the UI state and the underlying domain value.
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
|
@nadr0 Could you check the conflicts when you have a chance? Ty! |
issue
Provide a faster shorthand for common domains for our application
implementation
environment.txtor the domain input field so it will automatically highlight and sync the input field if they open the custom optionNew logo icons with bg stuff
Image updates