-
Notifications
You must be signed in to change notification settings - Fork 9
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
Adapt-1856: Misc v7 changes take 2 #808
Conversation
… remove unused inputBase variable in base.js
@@ -4,7 +4,8 @@ | |||
module.exports = function () { | |||
return { | |||
'none': '0', | |||
DEFAULT: '0.3rem', // form input border-radius |
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.
No more default? Why is this being removed?
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'm on the fence about this. It seems the 0.3rem is only used for input fields, so I feel like calling it our default might be a little misleading, but I could be convinced otherwise 😄
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.
https://tailwindcss.com/docs/border-radius#border-radiuses
I still think we need to provide a DEFAULT
option to tailwind. It is used in the su-rounded
class. You can choose the sensible default that you like but I think we should still have one.
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.
Sounds good - will have it =0.3rem then
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.
Thinking about this... I'm not completely sure about the 7px radius on the local footer login button. Will talk to @kerri-augenstein and see if I can convince her to maybe have this also equal to 0.3rem 😄 - but that's for later.
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.
GTG
READY FOR REVIEW
Summary
su-basefont-21
for cases where e.g., a non-21px base font size is set on the<body>
element but some section within the site need asu-basefont-21
override.last
andfirst
variants for margin since we often set different margins forfirst-child
andlast-child
, e.g., no margin-bottom on the last<li>
in a<ul>
https://tailwindcss.com/docs/hover-focus-and-other-states#last-child
Needed By (Date)
Urgency
Steps to Test
Affected Projects or Products
Associated Issues and/or People
@mention
them here)See Also