Skip to content
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

New website design #122

Closed
wants to merge 40 commits into from
Closed

New website design #122

wants to merge 40 commits into from

Conversation

muuvmuuv
Copy link
Contributor

Did not tested it yet on all devices since docusaurus is not very easy to modify and I just wanted to share a clickable version of my design in #121.

Related issue: #120
Related issue: #121

@Glavin001 Glavin001 mentioned this pull request Jul 12, 2018
Copy link
Member

@Glavin001 Glavin001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@muuvmuuv Right now this PR has 121 files changed.

I think we have too many new files under website/static/.
For example, both website/static/css/bootstrap.css and website/static/css/components/agolia.css should be installed via npm and/or use another tool. We don't need to have a copy of all of Bootstrap, we should have a dependency on Bootstrap version X and install it with npm install or yarn install.

I'm not sure about the others. Please let me know what you think. Overall, I do expect a lot of changes to the React components for the homepage and some custom CSS. However, 121 files is way too much. It indicates to me something -- like Bootstrap -- must be copied from somewhere else, when we really just need a reference and should not Git commit those files.

@Glavin001
Copy link
Member

BTW, since I previous said it in another chat and not here: this is awesome 🎉 !! Keep up the great work! Let me know if you need anything 😃

@muuvmuuv
Copy link
Contributor Author

@Glavin001 the reason why I added all manually is that I was not sure how to implement them right. Normally I use SCSS to include only necessary parts of bootstrap (or any other library) but since Docusaurus are not supporting SCSS I added it myself. Sure in future that would be changed (js libs would also be placed in package.json)!

Why are there so many files in /static: the reason is that I tried to split modules/components and parts of the templates in different files like I would do in SCSS. It is great that Docusaurus is parsing all CSS files, so it is working fine.

I really tried to keep it small, but also clean and readable. I think it just looks like many files for you. If you would dive deeper into the structure I made, it would make more sense to you.

PS: agolia.css is an override, not the full CSS (that comes from Docusaurus btw.)

- Styling fixes. (we should reconfigure the UbiCli that it adds EOL)
- prettier eslint/styleling
- added custom-select from bootstrap to OptionsDoc.ts
- beautification…
@todo
Copy link

todo bot commented Jul 12, 2018

remove this when bootstrap can load via cdn or node_modules */

/* TODO: remove this when bootstrap can load via cdn or node_modules */
.row {
width: 100%;
}


This comment was generated by todo based on a TODO comment in f701bd5 in #122. cc @muuvmuuv.

@muuvmuuv
Copy link
Contributor Author

We should also update the stats. I don‘t know where you get them from, but maybe we can pull them on demand? So a script that updates a JSON file every 10minutes/days(?) and the website is parsing them. Just an idea.

@Glavin001
Copy link
Member

Normally I use SCSS to include only necessary parts of bootstrap (or any other library) but since Docusaurus are not supporting SCSS I added it myself.

@muuvmuuv Even though Docusaurus does not support SCSS I do not mind adding some custom SCSS compiling steps to our pipeline.

This is the script Netlify uses to generate our website: https://github.com/Unibeautify/website/blob/master/scripts/netlify.sh#L12

I wouldn't mind if you setup something for SCSS such that running yarn build:static (or some other script name) would generate the appropriate files, pulling in from node_modules/ and such. You could use Webpack if you like, I am interested to see what you come up with 😃.

In the end, I cannot merge this with so many files. I need this process to be cut down and for dependencies to be from node_modules/.

Please let me know if you'd like to discuss more on this. We can work together on it. 😃

@muuvmuuv
Copy link
Contributor Author

Great, but I think for now CSS will work and fit.

Hm. Okay, I understand the reason of not merging that big PR, but how do I cut it down? Can I merge commits?

@stevenzeck
Copy link
Contributor

@muuvmuuv where did you get the CSS and JS files under static?

@muuvmuuv
Copy link
Contributor Author

@szeck87 what do you mean? I wrote the CSS and theme.js by myself, selectList.js and disqus.js are @Glavin001 and the rest you can ignore because we will use CDN instead or npm packages

@Glavin001
Copy link
Member

Glavin001 commented Jul 12, 2018

Can I merge commits?

Yes, you can squash commits together with Git. However, if you are unfamiliar with this functionality of Git, then I recommend create a fresh branch off of master then copying over the files you need and re-committing. Feel free to make a new PR and close this one if you do. Please be careful if you try to use Git Squash, as I would not want to lose all of this great progress! 😃

@szeck87 what do you mean? I wrote the CSS and theme.js by myself, selectList.js and disqus.js are @Glavin001 and the rest you can ignore because we will use CDN instead or npm packages

Sounds good 👍 . Looking forward to seeing it 🎉 .

@muuvmuuv
Copy link
Contributor Author

Ok, I‘ll make a new PR. Closing this one

@muuvmuuv muuvmuuv closed this Jul 13, 2018
@muuvmuuv muuvmuuv mentioned this pull request Jul 13, 2018
@muuvmuuv muuvmuuv deleted the new-website-design branch July 13, 2018 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants