-
Notifications
You must be signed in to change notification settings - Fork 164
Added a label to add last used date on each group #1028
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
base: develop
Are you sure you want to change the base?
Added a label to add last used date on each group #1028
Conversation
|
Important Review skippedAuto incremental 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 Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
WalkthroughChanges add a "last-used" feature to group cards. The lastUsedOnMs field is computed from group timestamp data, rendered as a formatted date element with an interactive tooltip displaying full datetime information, and styled with hover-triggered display behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
| Inconsistent null checking in timestamp conversion ▹ view | ||
| Inline Complex Data Transformation ▹ view | ||
| Duplicated Date Formatting Logic ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| groups/script.js | ✅ |
| groups/createElements.js | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
groups/createElements.js(2 hunks)groups/script.js(1 hunks)groups/style.css(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
groups/createElements.js (1)
groups/script.js (1)
group(278-278)
🔇 Additional comments (2)
groups/script.js (1)
199-204: Unnecessary defensive check;.secondsproperty never populated.The code checks for both
secondsand_seconds, but groups are loaded from the REST API endpoint (/discord-actions/groups) which consistently returns Firestore Timestamps in serialized format with_seconds. Thesecondsproperty (without underscore) is never populated. The check is valid defensive programming but the first condition is dead code and can be removed.Simplify to check only
_seconds:const lastUsedOnMs = group?.lastUsedOn?._seconds != null ? group.lastUsedOn._seconds * 1000 : undefined;groups/createElements.js (1)
47-67: Consider making locale configurable for internationalization.The date formatting uses a hardcoded
'en-US'locale in both the short and long date formats. For a more internationalized user experience, consider using the browser's locale or making this configurable.You could use the browser's default locale by omitting the locale parameter or using
navigator.language:-const shortFormatted = new Intl.DateTimeFormat('en-US', { +const shortFormatted = new Intl.DateTimeFormat(navigator.language, { weekday: 'short', year: 'numeric', month: 'short', day: 'numeric', }).format(date);And similarly for the tooltip formatting at line 58.
⛔ Skipped due to learnings
Learnt from: AnujChhikara Repo: Real-Dev-Squad/website-dashboard PR: 976 File: components/request-card/utils.js:15-27 Timestamp: 2025-04-14T12:08:38.070Z Learning: Date formatting functions in the website-dashboard project should consistently use 'en-US' locale for Intl.DateTimeFormat to maintain formatting consistency.
8e8946a to
7cfb954
Compare
… fixed:Inconsistent null checking in timestamp conversion
Date: 6 Nov 2025
Developer Name: @JAHANWEE
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes