Introduce a Reset button #12
Conversation
…alues The reset button allows users to reset all settings to their default values. When the button is clicked, all settings are reset to their default values.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Hey, @kodbilenadam ! Thanks for opening this PR. Here are a couple of things that I'd like to point out:
Reset Button
To best fit the existing UI, we have two options:
- We either create a small icon button and place it at the top of the settings, or
- We create a regular button and place it the
ActionsPanelcomponent above the other two existing buttons.
For either of these we should expand the Button primitive to add a variant that would adjust to this change. For the first option, I would either create a ghost variant for the first option and add the hover modifier later:
<Button variant="ghost" className="hover:bg-red-800/90" onClick={()=>resetSettings()} />For the second option, I would create a destructive variant and add it in the Actions Panel, as an actions button:
<ActtionButton variant="destructive" onClick={()=>resetSettings()} />Here are some guidelines for styling the button with TailwindCSS: https://play.tailwindcss.com/Rxj6yclXQS
If you do not feel comfortable using this framework and variant props, I can help you out.
Reset Settings
You are passing arbitrary values in this function, in config/defaults you can find the default values that are used by the state reducer. I would add these ones instead. It will be easier to maintain and understand what happens when clicking this button.
The huge huge thing to figure out in this PR is the onload value for: insetColor and isDark value because these are dynamically generated depending on the image.
This is important because when we click on the button, I would expect to get what I started with and these values are calculated in utils/color and applied in the callback in ClipboardImage component.
Maybe we can store them in a ref and use them when the reset button is applied? Wdyt? I'm open for ideas here. If this results challenging we can take it in a separate issue and address here only the UI and using the default default values as declared in config/defaults.
|
RE: Icons We are using Lucide for this project. Here's the Reset Icon reference in the Tailwind Play -> https://lucide.dev/icon/rotate-ccw |
|
Hey! Thank you for checking out PR! Here are my ideas on the things you mentioned:
I am using
I will make the necessary changes and update the code. I agree with you on this one.
Storing in ref and using them as a default value for reset button instead getting from I think creating another issue for storing those values will be much cleaner process. Let me change the default values and adding reset button with Settings text. Let's check deployed product and if we satisfy with result merge it to main close this issue and start implementing the storing values. Let me know your thoughts. @ekqt |
|
RE: Resetting Radio Button does not make in unchecked. If user clicks Reset button if the Radio button is checked on |
Agreed |
Not sure about that one from the top of my head but sounds like I must've left this as a uncontrolled input. Maybe ditching |
…ttings object The handleReset function has been refactored to use the defaultSettings object instead of hardcoding the default values. This improves maintainability and reduces the risk of errors when changing the default values. The value of the aspectRatio radio group is now set to the value of the settings.aspectRatio state variable to ensure that the correct value is selected when resetting the settings.
The default value of insetPadding has been changed from 0 to 1 to provide a better visual experience. The previous value of 0 caused the image to be too close to the border, which made it difficult to see. The new value of 1 provides a small amount of padding, which makes the image more visible and easier to work with.
This commit reorders the imports to follow the convention of having external libraries first, then internal modules. It also adds a new ghost variant to the buttonVariants constant.
…n icon The Settings UI layout has been improved by adding a title and a reset button icon. The title is now centered and the reset button is now an icon instead of text. This improves the overall look and feel of the Settings component.
|
I think with this commit we are ready to go live. What do you think? @ekqt |
The default value of insetPadding has been changed from 1 to 0 to remove the unnecessary padding around the image.
The ghost button style has been updated to have a slate background color and white text color. The hover effect has also been updated to have a red background color with 90% opacity. This change improves the visibility of the ghost button and makes it more consistent with the design system.
The insetPadding value was not being reset to the default value when the reset button was clicked. This fix sets the default value to 2, which is the desired value.
The hover effect and margin on the reset button were removed to simplify the button's appearance and make it consistent with the other buttons in the component.
This was fun! Thank you for the constructive feedbacks. 😄 |
|
@kodbilenadam great work! I'm planning on getting us a domain so we can make this project look a bit more official. Loving the ideas and the collaboration! Huge thanks! Make sure to recommend folks this web app whenever you see unframed screenshots haha Getting stars on GH will help us get traction. 🔥 |
Summary
The reset button allows users to reset all settings to their default values. When the button is clicked, all settings are reset to their default values.
Related to the: Introduce a Reset button
Bugs
When a user clicks the Reset button values of the settings get reset but the radio buttons are not updated. I tried some things but couldn't figure out the unspagetti way to do it.