search

jannis-baum / Vivify

Bring your Markdown to life
GNU General Public License v3.0
34 stars 4 forks source link

CSS refactor #151

Closed jannis-baum closed 2 months ago

jannis-baum commented 3 months ago

Close #120

jannis-baum commented 3 months ago

Hey @tuurep so I did this now, I think it made the CSS a lot less cluttered and will hopefully make it easier to maintain & extend in the future :)

I concentrated as hard as I could while doing this, I hope I didn't mess anything up ^^ If you want to check this I would recommend looking at each of the two commits whose messages are refactor(#120): extract ... colors individually, this way it should be easiest to figure out if I made any mistakes.

Also let me know if you're happy with this or if you would refactor anything else :) I did a couple of other small things, see commit messages

EDIT I forgot about the (single) Notebook color we have at the moment, added another commit that does that.

tuurep commented 3 months ago

Hey, looks great. Even surprises me how few variables we end up needing.

Tested and looks as expected on light theme. I only have naming-related concerns, I'll put a review.

tuurep commented 3 months ago

That's all!

Drawback of this is that we will have to come up with new variables and sort of maintain a naming scheme for these, but looks like it's worth it IMO.

jannis-baum commented 2 months ago

Absolutely no reason to apologize! I appreciate your dedication to getting these things right a lot, this is very important to the project. Thank you!

tuurep commented 2 months ago

Absolutely no reason to apologize! I appreciate your dedication to getting these things right a lot, this is very important to the project. Thank you!

Ok! Not always sure if I'm being counter-useful, so good to know :smile:

jannis-baum commented 2 months ago

Ok! Not always sure if I'm being counter-useful, so good to know 😄

Haha no, definitely useful, this is a big contribution to the quality!

I fixed the naming, you forgot to rename bold to muted in the Markdown styles :)

Now we're good right?

tuurep commented 2 months ago

Oh yeah, I forgot to rename the variables.. :facepalm:

Lets be sure that they're correct, careful because I changed the order of them (the old regular is not the new regular)

jannis-baum commented 2 months ago

Lets be sure that they're correct, careful because I changed the order of them (the old regular is not the new regular)

Ohhh okay haha, then they probably aren't correct. Can you look into it?

I'll be out for today, talk tomorrow! :)

tuurep commented 2 months ago

Yes, these just had to be flipped

Now it's good!

tuurep commented 2 months ago

I'll be out for today, talk tomorrow! :)

Okay, I see no reason not to merge now so I'll just go ahead and do it :D

Thanks