search

FNNDSC / ChRIS_store_ui

UI for the ChRIS Store
MIT License
49 stars 49 forks source link

Use CSS modules #151

Open jennydaman opened 3 years ago

jennydaman commented 3 years ago

https://create-react-app.dev/docs/adding-a-css-modules-stylesheet/

somya51p commented 3 years ago

@jennydaman Can you give a brief on this issue. Can I work on it?

jennydaman commented 3 years ago

The pf4 branch is a piece-by-piece rewrite of the ChRIS_store_ui. We want to make sure the code quality and maintainability is best.

In this issue, we want to evaluate the feasibility of using CSS modules. CSS modules are explained in the link above. Then, we want to refactor the project to use CSS modules.

jennydaman commented 3 years ago

There are currently 278 lines of code containing the word className in the codebase, so this is not a small issue.

somya51p commented 3 years ago

@jennydaman @mairin I wish to contribute to this issue. Can you please assign me the same.

jennydaman commented 3 years ago

Pointed out by @DivyanshiSingh there are new bugs introduced by #162, @somya51p could you take a look?

jennydaman commented 3 years ago

screenshot

jennydaman commented 3 years ago

@sonylomo fixed some of it in #182 which I just merged.

screenshot

somya51p commented 3 years ago

@jennydaman Sure, I will look into it.

DivyanshiSingh commented 3 years ago

@jennydaman I think we should have taken the changes piece by piece for every component and not at once. As now I can see there are some unwanted changes done to the previous merged PRs and also css bugs. Screenshot 2021-05-08 at 12 38 04 PM This is a merge issue which changed the behaviour of button on login page that I fixed earlier

2021-05-08-2

2021-05-08-1

2021-05-08

somya51p commented 3 years ago

@jennydaman @DivyanshiSingh CSS modules have to be specified separately for all className in CSS files. I figured it just now that a few className are missing in CSS files. So I am working on it.

DivyanshiSingh commented 3 years ago

@somya51p please fix the button issue that i have shown in the screenshot, you have accepted wrong changes while merging. Also here are some more css bugs - 2021-05-08-4 Footer space at the bottom

2021-05-08-3 Variation in card widths

somya51p commented 3 years ago

@DivyanshiSingh I am already working on the Dashboard part and the button issue for the Plugins page has already been resolved in #182. And I guess wrong changes have not been merged in my PR. But I will have a look into that.

DivyanshiSingh commented 3 years ago

And I guess wrong changes have not been merged in my PR. But I will have a look into that.

Through gitlens its showing your commit has the changes Screenshot 2021-05-08 at 12 38 04 PM

and the link in your commit https://github.com/FNNDSC/ChRIS_store_ui/pull/162/files#diff-6ab6d924a079befbe5cc4ba10cfd4bf8316d40008082f235501725bc3366a678L149

somya51p commented 3 years ago

@DivyanshiSingh Thanks for figuring out the glitch. I will look into it and also try to figure out how it happened.

jennydaman commented 3 years ago

@DivyanshiSingh your digging is so much appreciated, it is my fault for merging #162 prematurely without more testing.