servo / servo.org_2014-2020

Main website for Servo.
https://servo.org
Mozilla Public License 2.0
13 stars 31 forks source link

Replace/bump Bootstrap with Bootstrap Reboot #83

Closed ys-chung closed 4 years ago

ys-chung commented 4 years ago

I was going to bump the version of Bootstrap, as I did at #41. However the official customiser is not maintained anymore for v4, to customise it again would require modifying the variables in sass. Also because of changes in v4, it would require some work.

Considering above and that this site is relatively simple and only uses, aside from columns, the button and thumbnail components from Bootstrap. Therefore, in this PR I have made the following changes:

I understand that this change might be controversial, however I think this makes this page easier to maintain in the future as it does not depend on Bootstrap's class names anymore, as only its base styles are used.

Other language versions are unchanged in this PR.

Screenshot: image

SimonSapin commented 4 years ago

Thanks for the PR! I like how the code look nicer, indeed.

I notice that this uses display: grid instead of the float property to put things into two columns, which unfortunately is not yet implemented in Servo. So this would be a regression in terms of how well Servo can render its own website :)

As to making the site easier to maintain I don’t know if it’s really a concern. The site is very simple, and we only made 10 non-merge commits to this repository in the last year.

Finally, we are in the process of making large changes to Servo’s CSS layout implementation and have picked rendering servo.org correctly as an (arbitrary) milestone. This site is good for that because it is simple and it is mostly not a moving target. So now is not the best time to rewrite its stylesheet.

@jdm, what do you think?

ys-chung commented 4 years ago

Thanks for the quick response!

I have changed the thumbnail to use flexbox instead of grid, now it renders in Servo properly. For the "maintaining" part, as you have mentioned it does not require much maintenance, I simply meant updating Bootstrap, as what I first intended to do as a follow-up to that PR I mentioned in 2016.

jdm commented 4 years ago

Unfortunately while flexbox sometimes renders ok inside the default layout engine in Servo, we're also writing a new layout engine and it currently lacks flexbox support. I agree with Simon; in isolation these changes are a nice improvement, but since we don't actually update the site regularly and we rely on certain properties of the current implementation for high priority work, I don't think we're ready to accept these changes in their current state.