-
Notifications
You must be signed in to change notification settings - Fork 515
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
"🐛 Fix: Ensure weekdays are responsive on medium and small screens " #9747
base: develop
Are you sure you want to change the base?
"🐛 Fix: Ensure weekdays are responsive on medium and small screens " #9747
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Suggested Labels
Suggested Reviewers
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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
🧹 Nitpick comments (2)
src/CAREUI/interactive/WeekdayCheckbox.tsx (2)
41-41
: Remove extra spaces or trailing whitespace.
It appears there's an indentation or whitespace mismatch detected by the static analysis tool on this line. Cleaning it up eliminates unnecessary clutter, ensuring consistent formatting.- +🧰 Tools
🪛 eslint
[error] 41-41: Delete
··
(prettier/prettier)
70-70
: Remove extra blank line to satisfy prettier rules.
The static analysis tool indicates an unnecessary line break at the end of the file. Removing it aligns with coding style guidelines.- +🧰 Tools
🪛 eslint
[error] 69-70: Delete
⏎··
(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/CAREUI/interactive/WeekdayCheckbox.tsx
(3 hunks)
🧰 Additional context used
🪛 eslint
src/CAREUI/interactive/WeekdayCheckbox.tsx
[error] 41-41: Delete ··
(prettier/prettier)
[error] 49-49: Insert ,
(prettier/prettier)
[error] 69-70: Delete ⏎··
(prettier/prettier)
🔇 Additional comments (3)
src/CAREUI/interactive/WeekdayCheckbox.tsx (3)
38-38
: Good move to a grid layout for responsive design.
This change effectively improves the responsiveness of the WeekdayCheckbox component across various screen sizes, reducing layout breaks and improving the user experience.
46-46
: Nicely structured UI layout.
Usingflex flex-col
in tandem withitems-center justify-center
is a clean approach for centering elements. This improved organization helps maintain a straightforward user experience.
59-59
: Center-aligned label for cleaner UI
Centering the label text promotes consistency and a user-friendly layout. This is a solid improvement to accessibility and aesthetics.
isChecked | ||
? "border-primary-500 bg-white shadow" | ||
: "border-gray-300", | ||
: "border-gray-300" |
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.
💡 Codebase verification
The trailing comma is required after the ternary expression
The linting error is correct. The code shows a template literal using cx()
or a similar utility where multiple class strings are being composed. In this context, each item in the array (including the last one) should have a trailing comma according to your Prettier configuration. The ternary expression isChecked ? "border-primary-500 bg-white shadow" : "border-gray-300"
is the last item in the array and needs a trailing comma.
Here's the problematic code:
cx(
"flex flex-col items-center justify-center gap-2 rounded-lg border p-4 transition-all duration-200 ease-in-out",
isChecked
? "border-primary-500 bg-white shadow"
: "border-gray-300" // <-- Missing trailing comma
)}
🔗 Analysis chain
Verify the static analysis request for a trailing comma.
The static analysis tool reports a missing comma at line 49. However, this might be a false positive given the ternary expression. Double-check your prettier/eslint configs to confirm if a trailing comma is truly required.
Based on the linting output, I can see that there is indeed a Prettier error at line 49 in src/CAREUI/interactive/WeekdayCheckbox.tsx
that requires a trailing comma. Let me verify the surrounding context to ensure this is not a false positive.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify linting, focusing on missing commas or spacing issues around line 49
npm run lint || yarn lint
Length of output: 66359
Script:
#!/bin/bash
# Check the context around line 49 in WeekdayCheckbox.tsx
rg -A 3 -B 3 "border-gray-300" src/CAREUI/interactive/WeekdayCheckbox.tsx
Length of output: 378
🧰 Tools
🪛 eslint
[error] 49-49: Insert ,
(prettier/prettier)
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.
lgtm, do fix the coderabbits comment, discard lockfile changes and check the lint fixes....
also next time, do ensure the issue gets assigned before you work on it to avoid duplication of efforts. i had already made it responsive along with a bunch of other fixes and validations.... ignoring since this is your first time contribution 😀
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.
discard the lockfile changes
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.
Thank you for the feedback! I’ll address the CodeRabbit comments, discard the package-lock.json changes, and verify the lint fixes. I appreciate the reminder about assigning issues before working on them to avoid duplication. This is my first time contributing to open source so i apologise for my misatkes.
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.
No worries. Welcome to the community! Checkout https://docs.ohc.network and join our community at https://slack.ohc.network/
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.
i have already joined the community . I have fixed lint issues and discard lockfile changes should i push(i used npx prettier --write . which reformats entire codebase and also caused 400 commits) ?
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.
yup, need not ask btw 😅
…#9738)"
Here’s how you can format the changelog entry for your code changes:
Proposed Changes
Fixes #9738
Feature: Implemented WeekdayCheckbox component to allow selecting multiple weekdays with a grid layout.
UI Update: Changed layout from a flex-based design to a grid-based design for better structuring of days.
Styling: Adjusted padding (px-8 py-6) and border styling to ensure consistent checkbox appearance.
Label Alignment: Ensured labels are centered and aligned across all screen sizes for a cleaner UI.
Code Optimization: Removed redundant class declarations for improved clarity and readability.
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Add specs that demonstrate bug / test a new feature.
Update product documentation.
Ensure that UI text is kept in I18n files.
Prep screenshot or demo video for changelog entry, and attach it to issue.
Request for Peer Reviews
Completion of QA
Summary by CodeRabbit
WeekdayCheckbox
component layout to use responsive grid design