-
Notifications
You must be signed in to change notification settings - Fork 34
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
Added support for colored list indices and bullet points #1889
Added support for colored list indices and bullet points #1889
Conversation
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.
LGTM! Seems to work really well in my testing, I didn't come across any issues. Good work!
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.
Works great! I just had one question about a package
@@ -14282,6 +14282,11 @@ url@^0.11.0: | |||
punycode "1.3.2" | |||
querystring "0.2.0" | |||
|
|||
use-debounce@^7.0.0: |
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 needed?
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.
That is weird. I didn't install it. Is it possible that somehow it installed it when I ran yarn install
when I cloned the repo?!
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.
It's possible (and likely lol) that it might be a mistake on a previous PR I made, and neglected to commit the yarn.lock file. I'm okay merging it in then
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.
It's up to you. I can try downgrading it if you want me to.
I had to change my approach a couple times to facilitate testing.
Besides all the precedence CSS/Sass I dealt with, I ended up creating a new function on obo's
MockElement
class to give us the supposed color of list indices and bullet points (right now, in order to be more careful and as expected, this function only works with list elements with text inside since we may have other types of elements as list elements).As always, I'd be glad to address any requested changes.
Fixes #1763