feat: opt-in repeated pulse animation on download graph#2121
feat: opt-in repeated pulse animation on download graph#2121graphieros merged 7 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a new boolean setting Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 834aed70-74bf-47ff-b09f-b5a58784b333
📒 Files selected for processing (5)
app/components/Package/WeeklyDownloadStats.vueapp/composables/useSettings.tsapp/pages/settings.vuei18n/locales/en.jsoni18n/schema.json
|
I think you should also update the settings state when using the easter egg |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
I don't quite fully understand what the easter egg is, could you explain what would need updating there, please? 🥺 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
i18n/locales/en.json (1)
141-142: Align graph terminology in the setting label and description.Line 141 says “mini graph” while Line 142 says “weekly download graph”. If this is the same feature, use one term consistently to reduce ambiguity in Settings.
✏️ Suggested copy tweak
- "enable_graph_pulse_loop": "Enable looping of pulse effect on mini graph", + "enable_graph_pulse_loop": "Enable looping pulse effect on weekly download graph",
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cfe10d27-01f8-4d27-8706-93185820b25a
📒 Files selected for processing (1)
i18n/locales/en.json
Oh of course, sorry. So if a user happens to use the easter egg, the state in the settings can be out of sync. |
|
Is this what you would also expect or do you think it should always activate when hitting the combination? |
I'm sorry I edited by mistake your comment. More like toggling through the easter egg changes the settings state too, so it is in sync and remains. Wdyt ? |
knowler
left a comment
There was a problem hiding this comment.
I’m curious if instead of creating a specific settings for this, if maybe we should create one called “Whimsical mode” or “Whimsy mode” which we could group future features under, so that we have a bit of a better contract with the user. In the description, we could still list this feature as an example, but also should note that some features might be distracting.
I agree, the setting could be more broad indeed |
knowler
left a comment
There was a problem hiding this comment.
LGTM… we discussed on Discord and we might group this under another toggle in the future, once we come up with a good name for it.
Co-authored-by: Alec Lloyd Probert <55991794+graphieros@users.noreply.github.com>
🧭 Context
As initially suggested on Discord, it would be nice to bring pack the looping pulse animation of the weekly download graph. Because it is not accessibly, we leave it disabled by default, and allow users to opt-in with a new setting.
📚 Description
Adapted the
WeeklyDownloadGraph.vuecomponent (pinging @graphieros as they are the export of their component ❤️ ). Additionally introduced a new setting, new text strings inen.jsonetc.Also pinging @knowler to review this change for accessibility please 🙏