-
Notifications
You must be signed in to change notification settings - Fork 95
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
Updates color in nutrition info and removes bullet point #511
Conversation
…bullets Signed-off-by: Sebastian Fey <info@sebastianfey.de>
Signed-off-by: Sebastian Fey <info@sebastianfey.de>
Codecov Report
@@ Coverage Diff @@
## master #511 +/- ##
========================================
Coverage 0.90% 0.90%
Complexity 416 416
========================================
Files 13 13
Lines 1319 1319
========================================
Hits 12 12
Misses 1307 1307
Flags with carried forward coverage won't be shown. Click here to find out more. |
src/components/RecipeView.vue
Outdated
@@ -379,6 +379,10 @@ aside { | |||
margin-top: 2rem; | |||
} | |||
|
|||
.nutrition-items { | |||
list-style-type: none !important; |
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.
Is this really needed? I am always a bit suspicious when !important
is in use.
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 other ways would be to either
- change the definition of
#app-content-wrapper aside ul
in the mainstyle.css
since this has a higher specificity than using the class - use
#app-content-wrapper aside ul.nutrition-items
which might be just as bad as using!important
:D
For 1. I need to test if there are any unintended consequences when removing the definition. But changing it there might be a sensible way to handle this.
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.
See #515. The global CSS is a bad idea to change the styling of things within vue.
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.
For now, I suggest keeping the !important
and opening an issue to remind us to come back to it eventually once #515 is finished.
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 forgot to push the updated version. We can start deprecating the global style here rather than introducing new code "depending" on the old style file which needs to be changed in the future.
Signed-off-by: Sebastian Fey <info@sebastianfey.de>
I am going to merge here. If something broke by this, we might need to tweak it at appropriate locations though. |
Closes #510