vega / editor

Editor/IDE for Vega and Vega-Lite
https://vega.github.io/editor/
BSD 3-Clause "New" or "Revised" License
163 stars 87 forks source link

Refactor CSS of components #445

Open domoritz opened 5 years ago

domoritz commented 5 years ago

Right now we use a lot of custom classes. Instead, we should have a single class for a component (e.g. help-modal) and then use rules that are for objects nested below (e.g. .help-modal p a).

nik72619c commented 4 years ago

I'll give it a try :)

domoritz commented 4 years ago

Sweet, thanks.

nik72619c commented 4 years ago

I'm not quite able to get clarity on refactoring css here...any examples of how is it to be refactored @domoritz ?

domoritz commented 4 years ago

Sure! The main problems I see is that we have a lot of repetition across files, we don’t have a good way of name spacing properties right now (the styles in one file could affect other components), and we don’t use consistent style for similar components.

I think we want to reduce the number of classes and make the existing ones more meaningful and reusable. Similarly, we should use css variables for colors. We could use tachyons as a css framework to simplify our style.

Working on this will mean that you have to look across files and refactor.

Does this make sense?

rav1kantsingh commented 4 years ago

I would like to help on this issue. Is there any branch for this issue?

domoritz commented 4 years ago

Welcome to Vega! Not yet. Feel free to send a pull request. I recommend starting with one component and then going from there.

rav1kantsingh commented 4 years ago

As you mentioned to use CSS variables for colors. should I define color variables on top of same CSS file or in some other file?

domoritz commented 4 years ago

Let's define them in the main CSS file. If you work on variables first, let's replace all colors at once.

PunitLodha commented 4 years ago

Hey @domoritz . I am new to Open Source. I would like to work on this issue. Should i replace the classes by the tachyons classes?

domoritz commented 4 years ago

That's a lot of work. Maybe let's start by just refactoring the CSS. Right now, we have a lot of duplications and few clear separations. We can switch to a framework once you have had some exposure to our current style.

rav1kantsingh commented 4 years ago

I am also open to this work. Please let me know how can I help.

domoritz commented 4 years ago

Have a look at the code and send PRs to fix issues you see.

PunitLodha commented 4 years ago

I have sent a PR. Please review it. If its good, then i shall start working on the other components

PunitLodha commented 4 years ago

Should we start using BEM for structuring the CSS?

domoritz commented 4 years ago

I'm leaving this decision to you as long as we overall increase consistency.

adrijshikhar commented 4 years ago

@domoritz is this issue still open? I would like to take this.

domoritz commented 4 years ago

Yep, the CSS can always be improved.

adrijshikhar commented 4 years ago

can you give me an example of how you want it to be?

domoritz commented 4 years ago

Not really. Just look through the code and notice how we could clean up the CSS. I know, this issue isn't very concrete.

ashu8912 commented 4 years ago

Hey @domoritz I am working on this also are we planning to bring theming feature to the editor in future? coz than we will have to make css refactors while keeping that in mind.

domoritz commented 4 years ago

I don't think theming is a priority.

ashu8912 commented 4 years ago

ok for now all the modal have distorted css working on fixing that within one pr