RetroShare / RSNewWebUI

30 stars 20 forks source link

feat: migrate entirely from css to scss #71

Closed zelfroster closed 1 year ago

zelfroster commented 1 year ago

Closes #70

zelfroster commented 1 year ago

I have now defined two scripts in package.json, One for development purpose and other for minifying and building so that CSS build size is low and easy to load for the application.

I am done migrating CSS to SCSS and code refactoring is left, which can be done later in a different PR.

This PR can be merged : ) Please have a look at it @csoler @defnax @rottencandy

rottencandy commented 1 year ago

I don't feel very confident about commiting a build artefact to the repo, but there doesn't seem to be any other way to be able to use scss.

rottencandy commented 1 year ago

With this, all contributors will need to run scss and commit the updated build file for any css-related change. @zelfroster can you also update the readme section with instructions?

zelfroster commented 1 year ago

With this, all contributors will need to run scss and commit the updated build file for any css-related change.

Actually, Since I have included the app.css itself in the repo, they won't need to build the scss. If they want to change the scss then only they will have to build it. As of now, the built version of app.css is already in the repo and when the user runs qmake . , The build script for respective OS will just copy that app.css file to the webui.

Please correct me, as I think I haven't understood correctly

rottencandy commented 1 year ago

Yup, that's what I meant :slightly_smiling_face:

Anytime we change any of the scss files, we will also have to commit the updated app.css changes.

zelfroster commented 1 year ago

Yup, that's what I meant :slightly_smiling_face:

Anytime we change any of the scss files, we will also have to commit the updated app.css changes.

Yes yes, 😅 But Even without scss we would have to commit the css files as well. And I can't think of a better way for now 😅

defnax commented 1 year ago

@rottencandy @csoler ready for merge?

rottencandy commented 1 year ago

I'm fine with it, but would be great if we could update readme instructions as well.

zelfroster commented 1 year ago

I'm fine with it, but would be great if we could update readme instructions as well.

Sure, I will do it. But I am also thinking of doing one more thing which is using a bundler like parcel for bundling the whole webui.

So, should I go for it?

One more thing, I was getting some issues when using the API endpoints to get data in webui. Who will be the best person to discuss it with? I am asking so I don't disturb anyone unnecessarily 😅

rottencandy commented 1 year ago

I think you can discuss with @csoler

zelfroster commented 1 year ago

I think you can discuss with @csoler

Sure 😁 and I will update the readme later, so please merge this PR.