-
Notifications
You must be signed in to change notification settings - Fork 112
core: frontend: components: kraken: StoreExtensionCard: Fix architecture list color #3382
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
core: frontend: components: kraken: StoreExtensionCard: Fix architecture list color #3382
Conversation
Reviewer's GuideThis PR removes the dynamic inline styling for the architecture list and cleans up the component's CSS by deleting the computed style property and the hardcoded white color rule to fix the list color. Class diagram for StoreExtensionCard component after style cleanupclassDiagram
class StoreExtensionCard {
+String compatible_architectures
+String extension_company()
+String update_button_text()
-architecture_list_style() // removed
}
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 @patrickelectric - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
We have another problem that the extension page is not being able to fetch the logos colors, that is why the texts started to mix with the tag color, so I think we need to first fix the other problem and check if this still happens. |
I think that a good fix will be instead of hard coding it to work with light theme just modify to fallback, since will keep both existing behavior and provide a fallback in case image do not load, also the other changes on code will not be needed. architecture_list_style(): Record<string, string> {
if (this.card_dominant_color === undefined) {
return {
color: this.$vuetify.theme.dark ? 'white' : 'black',
}
}
return {
color: this.is_card_dominant_color_light ? 'black' : 'white',
backgroundColor: this.card_dominant_color,
}
}, |
…_color is undefined Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com> Co-authored-by: Joao Mario Lago <joao.mario.lago@hotmail.com>
efcb1e7
to
19efffd
Compare
@joaomariolago check now |
before:
after:
Summary by Sourcery
Fix the architecture list color by removing the outdated dynamic styling and default CSS color declarations
Bug Fixes:
architecture_list_style
computed property and its style binding from StoreExtensionCard.vuecolor: white
rule from the.architectures-list
CSS to allow correct text coloring