-
-
Notifications
You must be signed in to change notification settings - Fork 789
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
Make toolkit guide cards WCAG compliant #3726
Make toolkit guide cards WCAG compliant #3726
Conversation
Want to review this pull request? Take a look at this documentation for a step by step guide! From your project repository, check out a new branch and test the changes.
|
@plang-psm Did you get a chance to slack michaelmagen about your findings? If not, feel free to post them here, if you already spoke with him, don't worry about it. |
Hi @blulady , just wrapped up our meeting regarding the issue! @michaelmagen will be finishing up and submitting for PR review soon. |
I can't wait to see how you figured out the spacing! |
I added the changed I had discussed with @plang-psm. However, there is an extra commit called "Merge brach 'hackforla:gh-pages' into guide-card-wcag-update-2929" from when I tried to incorporate the update my topic branch with the changes made in the hackforla repository. Is this an issue? |
Shouldn't be. |
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.
The issue linked says to make it look like the site. I would argue that you have made it look better than the site, especially for the Responsive view. All of the mobile views seem to have the problem of the default card not being the same size as the website and while, personally, I think it's fine, I am uncertain if it is functioning as designed.
The ipads look good. Until you come to the view of the Surface Duo which seems to have the same issue but the Galaxy Fold has the same problem
The rest look amazing. All of which Matt said way more succinctly.
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 completely agree with @blulady 's argument that your changes look better than the live site for the small to medium responsive views. Furthermore, you did a great job communicating with the team, asking for help, and fine tuning your solution.
Amazing work on this issue @michaelmagen ! 👍
Fixes #2929
What changes did you make and why did you make them ?
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
Visuals before changes are applied
Visuals after changes are applied
Figma design for the Toolkit page