-
Notifications
You must be signed in to change notification settings - Fork 414
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
Newnew #2144
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.
You might be starting to dislike me, but some more comments.
I think these are mostly minor alignment/consistency issues. The code is looking way better (re media-tile media-card).
I think the biggest outstanding issue is the icons not using the Icon
component.
I know there will always be stuff that can be fixed so I don't want to have this sit in review forever. I think we can merge this after these issues are addressed then fix any other stuff individually.
Created a new PR for #2102 because that had so many comments.
PR Checklist
PR Type
What kind of change does this PR introduce?
Fixes
I borked #2053 while attempting to have a sane rebase. This PR refactors the CSS and makes semantic changes in the rendered HTML.
Closes #1976.
What is the current behavior?
Nothing the average user would notice, aside from styling.
What is the new behavior?
New hotness. Sass is (more) organized and easier to manage. It still isn't perfect but it's close.