-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fix horizontal scroll bug #35
Conversation
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.
@IsaacMilarky and @natalialuzuriaga should give this a look also now that we're all back. projects.liquid
looks legit, but I'm not sure about repos.js
. Thoughts?
site/_data/repos.js
Outdated
"EASi is a web application supporting the IT governance process at CMS.", | ||
category: "apps", | ||
}, | ||
// "CMSgov/easi-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.
Are we just commenting out this one entry?
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.
This entry messes up the script for some reason, I figured the repo was set to private or deleted
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.
Aha! I figured it out I reckon. Seems as though the repo has been transferred between CMS orgs! It is now hosted at https://github.com/CMS-Enterprise/easi-app!
easi-app is one of our most active oss projects, so we don't want to just skip or ignore it. For now, could we just manually update/rename the repos.js
entry with the new URL/path, and move the associated _data
accordingly?
@IsaacMilarky This seems like it will be a relatively rare occurrence, but an edgecase I suppose we should check for and catch? Let's ship this bugfix for now as-is, but file a new ticket for "Check for whether a repo has been transferred between CMS GitHub orgs."
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.
There is an API endpoint to check to see if a repository was moved and it returns the new location so we could set up a method to check for that.
The endpoint is in the format https://api.github.com/repos/<repo_group>/<repo_name>
and will tell you the new location if the repository has been moved.
For now we can just rename it but we could have some additional resolution logic in theory.
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.
LGTM +1 🚢 🐛 🔧
@Firebird1029 yup you are completely right with the deployed website being an outdated build. For some reason, the |
@natalialuzuriaga Something weird is happening -- looks like none of the bugs are fixed at https://dsacms.github.io/open/projects/ but the |
Fix horizontal scroll bug
Problem
On mobile, there is a horizontal scroll caused by text that overflows the cards. By adding an overflow CSS property, this removes the issue. Some other compile-time issues were also resolved.
Solution
Also, some of the issues brought up in #23 do not appear when the website is tested locally, such as the menu. The deployed website at dsacms.github.io/open is probably just an outdated build.