-
Notifications
You must be signed in to change notification settings - Fork 119
Add color background per version type in version-chooser #3512
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: master
Are you sure you want to change the base?
Add color background per version type in version-chooser #3512
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideVersion cards now feature a dynamic gradient background colored by version type (stable, beta, or other) via a new computed property and utility functions, along with a minor spacing adjustment. Class diagram for updated VersionCard component and Utils functionsclassDiagram
class VersionCard {
+backgroundColor(): string
asTimeAgo(value: string)
}
class Utils {
+isStable(version: string): boolean
+isBeta(version: string): boolean
}
VersionCard --> Utils: uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- Consider using a semantic‐versioning library (instead of regex) to more accurately classify stable/beta versions and avoid accidentally misclassifying patch or prerelease tags.
- Rather than inlining the gradient style, extract it into named CSS classes or theme variables to improve maintainability and keep the template cleaner.
- The change from
my-4tomy-1affects card spacing—please verify this aligns with the intended design and remains consistent across breakpoints.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using a semantic‐versioning library (instead of regex) to more accurately classify stable/beta versions and avoid accidentally misclassifying patch or prerelease tags.
- Rather than inlining the gradient style, extract it into named CSS classes or theme variables to improve maintainability and keep the template cleaner.
- The change from `my-4` to `my-1` affects card spacing—please verify this aligns with the intended design and remains consistent across breakpoints.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
The idea is awesome and will help a lot when picking a version. It would be nice to try sticking more to the current color palette. Probably @ArturoManzoli or Elisa can help. |
ES-Alexander
left a 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.
Nice update!
It'd be nice to also add a text description of the release stabilities1 (like was discussed in the meetings), but I suppose that could also go in a separate PR 🤷♂️
@rafaellehmkuhl the PR is using dynamic colours from the BlueOS theme (which are also used in the failsafe diagrams, and the user can override if they want to). Whether those colours should be changed is out of scope for this PR.
Footnotes
-
Could be something like "Stable releases are validated and tested, and recommended for most users. Beta releases contain bug-fixes and new features, but may not have been extensively tested. Development releases may be untested, and are only suggested for briefly testing specific development changes. See the documentation for more details." ↩
| <v-sheet | ||
| class="d-flex flex-column align-center my-4 pa-4 flex-sm-row py-sm-0" | ||
| class="d-flex flex-column align-center my-1 pa-4 flex-sm-row py-sm-0" | ||
| :style="{ background: `linear-gradient(135deg, #1E1E1E 20%, ${backgroundColor} 100%)` }" |
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.
| :style="{ background: `linear-gradient(135deg, #1E1E1E 20%, ${backgroundColor} 100%)` }" | |
| :style="{ background: `linear-gradient(135deg in oklab, var(--v-outline-base) 20%, ${backgroundColor} 100%)` }" |
oklab and oklch are in baseline CSS, so we may as well use perceptually linear colour spaces where we can, at least for transitions/gradients.
I'm defaulting to oklab here because it's slightly more intuitive as a transitive space, but oklch provides more vibrant (/less grey/muddy) gradients at the "cost" of occasionally introducing interim colours as it sweeps around the hue wheel, so is also worth considering 🤷♂️
Per my earlier review comment, the starting colour should account for the light/dark mode status. I'm unsure whether this is the best approach, but it does use a variable that's already used elsewhere in the software (e.g. for failsafe icon outlines), so at least avoids defining something completely new to maintain and document.
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.
Thanks, the outline-base did not work, I moved to transparent and the result is much better.
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.
@ES-Alexander check the new images in the PR
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.
the outline-base did not work
Curious - I wonder why... Ahh yeah, it's the opposite side of the lightness spectrum, so the text gets hidden.
I moved to transparent and the result is much better.
Fair enough - nice! :-)
check the new images in the PR
@patrickelectric it looks good, but the "Running" indicator is seemingly hard to read in light mode, and I expect it would be even harder to read on stable versions (where the background is green). Maybe the background of the "Running" rectangle should be made less transparent, and possibly less green?
Signed-off-by: Patrick José Pereira <[email protected]>
…ison Signed-off-by: Patrick José Pereira <[email protected]>
767e7b9 to
680000c
Compare
|
@dgudiel mentioned the appearance of this isn't super consistent with other parts of the interface. The suggested alternative is to add stability chips (e.g. "stable", "beta", "dev"), like GitHub's version labels: |


Summary by Sourcery
Add dynamic background gradients to version cards based on version type, introduce a beta version checker, and adjust card spacing.
Enhancements: