-
Notifications
You must be signed in to change notification settings - Fork 325
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
improve ui in filters #1884
improve ui in filters #1884
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe Changes
Assessment against linked issues
Poem
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 as PR comments)
Additionally, you can add 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.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
src/User/components/Popular_Categories/Filters.jsx (1)
Line range hint
13-13
: Correct the style attribute in theaside
element.The
filterStyles
object is being used incorrectly. It should be directly applied, not wrapped in curly braces within the style attribute.- style={{ filterStyles }} + style={filterStyles}
<h3 className="font-semibold mb-2">{title}</h3> | ||
<select | ||
onChange={onChange} | ||
className="w-full p-2 border rounded-lg bg-[#166635ff] text-white hover:bg-[#3d9907ff] focus:bg-[#bad9a8] focus:outline-none" | ||
style={{ borderRadius: '1rem' }} | ||
> | ||
<option value="" className="bg-[#166635ff] text-white hover:bg-[#3d9907ff] focus:bg-[#bad9a8]"> | ||
All | ||
</option> | ||
{options.map((option) => ( | ||
<option | ||
key={option} | ||
value={option} | ||
className="bg-[#166635ff] text-white hover:bg-[#3d9907ff] focus:bg-[#bad9a8]" | ||
style={{ borderRadius: '1rem' }} | ||
> | ||
{title === "Price" ? `Under $${option}` : option} | ||
</option> | ||
))} | ||
</select> |
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.
Refactor repeated styles and classes for maintainability.
The styles and classes related to the background color, text color, hover, and focus states are repeated across multiple elements. Consider using a CSS class to manage these styles centrally.
- className="bg-[#166635ff] text-white hover:bg-[#3d9907ff] focus:bg-[#bad9a8]"
+ className="filter-option-style"
And define .filter-option-style
in an external CSS file with the appropriate properties.
Approve the inline style for rounded corners.
The inline style for borderRadius
is well-placed since it allows easy customization for specific elements.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<h3 className="font-semibold mb-2">{title}</h3> | |
<select | |
onChange={onChange} | |
className="w-full p-2 border rounded-lg bg-[#166635ff] text-white hover:bg-[#3d9907ff] focus:bg-[#bad9a8] focus:outline-none" | |
style={{ borderRadius: '1rem' }} | |
> | |
<option value="" className="bg-[#166635ff] text-white hover:bg-[#3d9907ff] focus:bg-[#bad9a8]"> | |
All | |
</option> | |
{options.map((option) => ( | |
<option | |
key={option} | |
value={option} | |
className="bg-[#166635ff] text-white hover:bg-[#3d9907ff] focus:bg-[#bad9a8]" | |
style={{ borderRadius: '1rem' }} | |
> | |
{title === "Price" ? `Under $${option}` : option} | |
</option> | |
))} | |
</select> | |
<h3 className="font-semibold mb-2">{title}</h3> | |
<select | |
onChange={onChange} | |
className="w-full p-2 border rounded-lg bg-[#166635ff] text-white hover:bg-[#3d9907ff] focus:bg-[#bad9a8] focus:outline-none" | |
style={{ borderRadius: '1rem' }} | |
> | |
<option value="" className="filter-option-style"> | |
All | |
</option> | |
{options.map((option) => ( | |
<option | |
key={option} | |
value={option} | |
className="filter-option-style" | |
style={{ borderRadius: '1rem' }} | |
> | |
{title === "Price" ? `Under $${option}` : option} | |
</option> | |
))} | |
</select> | |
``` | |
Additionally, you would need to define the `.filter-option-style` class in an external CSS file: | |
```css | |
.filter-option-style { | |
background-color: #166635ff; | |
color: white; | |
border-radius: 1rem; | |
} | |
.filter-option-style:hover { | |
background-color: #3d9907ff; | |
} | |
.filter-option-style:focus { | |
background-color: #bad9a8; | |
} |
@codervivek5 pls review this |
@codervivek5 pls review |
make is same color not like multi color |
@codervivek5 i dont understand what youre saying, pls specify further |
@Priyal208 is attempting to deploy a commit to the Vivek Prajapati's projects Team on Vercel. A member of the Team first needs to authorize it. |
@codervivek5 pls merge this |
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/User/components/Popular_Categories/Filters.jsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/User/components/Popular_Categories/Filters.jsx
there is no need to make any changes to this dashboard page...if needed then I'll raise a new issue |
actually filter is not in the right place or not seem like working correct..... there is some bugs in filter so at this time we can't add your PR for these changes. you can find another issue then let me know if that will needful then definetly assgin to you I can understand of your effort but sorry for now. |
#1851 fixed
Close #1851
Changes proposed
I improved UI of filters section, changed colors also changed them specifically for hover and focus, as you can see in the below image , on hover there is different color and when one section is focused i.e open color changes, I also give some curvature to the rectangular div to make it look more appealing
Thus i made the UI better beautiful and more attractive
Screenshots
Summary by CodeRabbit
Summary by CodeRabbit