-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Migrate Jekyll website from Less to Sass #565
Conversation
✔️ Deploy Preview for keen-clarke-470db9 ready! 🔨 Explore the source changes: 7c78c09 🔍 Inspect the deploy log: https://app.netlify.com/sites/keen-clarke-470db9/deploys/611202d9e1748e0009a11db9 😎 Browse the preview: https://deploy-preview-565--keen-clarke-470db9.netlify.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this, Rob! Our public website tech is outdated, and my thought has been that at some point we'll create a new website altogether, maybe in the next few months. In the meantime, these updates are a big productivity boost.
It is my assumption if the Platform team ever uses a CSS preprocessor in Aperture it will likely be Sass.
We use Tailwind for all our CSS, and it pretty much removes the need for any CSS pre-processing, so my understanding is that we don't use either Sass or Less right now and will probably rely on a small number of PostCSS plugins.
QA
Thanks for proposing a QA checklist. One curious difference I noticed in the IaC Library was this:
Yes, as you pointed out the search bar color is off, but also there's now a repo count? It's a nice change, but I'm curious why it doesn't show up in prod?
I also found this bug, where the open arrow is pointed in the wrong direction, but it looks like that exists in prod, too, so we should probably file a separate GitHub issue for that.
I found a bug where the magnifying glass icon didn't render on the search bar for the guides:
On that same page, the newsletter sign up at the bottom wasn't centered:
The guides themselves look pretty good.
Once the above items are fixed, I think this is ready to ship.
Hi @josh-padnick, Thanks for the review! Most importantly I figure you are still using x86. Did you try to build the Docker image? Did that work? The Netlify builds look okay, so I'm assuming it works properly.
That is exactly my understanding. I looked at the hot new JS tech you guys are using and realized there probably needs to be an interim step - remain in Jekyll, Bootstrap 3.7 etc.
Yeah, I had a look at that and figured you'd avoided CSS preprocessors altogether. I thought it was interesting Tailwind only uses PostCSS. Then again I'm not really up to date on the latest frontend tech, so maybe its a new trend.
It seems I've found a repo count on production as well, so I've left it for now. I figured it's not worth the effort to start digging into JS changes on this branch.
That seemed to be an easy fix, so I went ahead and did that one. Let me know if you run into any other issues as I've commented out a selector property.
I've fixed the font path for the bootstrap fonts. It seems to render correctly now.
I've also figured out what was causing that. It seemed to be related to a One other visual issue, I've found is related to the 'filter' button on the guides page. In the interest of time, I've also avoided digging into that. It seems to also be happening on prod. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a quick scan of the preview website and everything LGTM!
Fixed in above comment. Please re-review.
@brikis98 oh wicked! This PR will significantly help my Dev environment and hopefully others. |
This PR changes our website technology stack to use Sass instead of LESS. (Note: Scroll to the bottom for the technical details). In a nutshell, it turns this:
Into this:
Why you might ask?
less.rb
hasn't had much love for ~7 years: https://github.com/cowboyd/less.rbWhat's the catch?
Technically we are updating one deprecated library -
less.rb
for another: jekyll/jekyll-sass-converter#116, although they'll likely upgrade to the Dart version of Sass in the future.Technical Details
This PR implements the following major changes:
less2scss
NPM package to convert our stylesheets to Sass.sassc
.2.7.4-buster
.Known Issues
QA
(Whoever / @josh-padnick) Mark these off once you've done a 1:1 visual diff against gruntwork.io.