-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
refactor: inline colour picker #1412
refactor: inline colour picker #1412
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice and straight to the point. I believe we need to make some small changes to improve the feature UX
- could we show the selected colour on the background of the
InlineColourPicker
element - could we show the colour code in the input field of the
SwatchPicker
- the input and the colour picker seem to be out of sync (see screenshot), can this be improved?
apps/client/src/common/components/view-params-editor/InlineColourPicker.tsx
Outdated
Show resolved
Hide resolved
I'll work on this and let you know |
hey. firstly, a very happy new year to you :)) secondly, found the issue for the 3 points that was mentioned. Needed to add another prop to handle the way we were passing in the color from pushing the recent changes, let me know if you'd like any other changes. attaching a video to show the working ontime-pr-2-update-1-2025-01-02_17.42.21.mp4 |
…gpal/ontime into refactor/inline-colour-picker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, really good, thank you for your contribution!
If you are willing to continue contributing, I would like to find you something a bit more involved.
Can you help me understand what is your preferred task setup? Do you like to work fullstack? Do you prefer react tasks? Do you like working with technical or performance tasks?
Either way, thank you again and happy new year!
EDIT: I saw that the border radius of the elements inside the popover did not match the popover so i thought I would fix it
I didnt really manage and decided it was not worth sinking too much work on this, but made some changes that you are welcome to comment on. #1420
Hope this is OK with you and that you dont feel I am redoing your work
i don't feel that way at all. i'm glad to know that there were improvements that were possible. i believe the product should have the best code possible. super glad i could contribute in the first place
i'd love to continue. i've wanted to work on performance tasks for quite a while, other than that i like to work fullstack as well. so i'd be good with any of the tasks. meanwhile i'll understand the codebase better on my end |
Do you use discord? if so, maybe hop into discord server and find me as cv_ and we can find something together Otherwise, I had in mind something slightly more complex but with little domain requirements (all of these are UI only)
If you want to continue contributing in long term (with no obligation). I can also add you to the contributors list to simplify the git flow |
just joined the server. i think we're already in dms.
i'd like to keep contributing, i think it'd be better if we continue with the current flow. i feel i should know more things and once i'm much more familiar with the codebase and some newer concepts, if it extends to long term, would love to be added to the list |
Changes
<PopoverPicker>
with<SwatchPicker>
setColour
as the onChange callback, which is already being debounced inside<SwatchPicker>