-
Notifications
You must be signed in to change notification settings - Fork 111
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
button lift and text shadow #80
button lift and text shadow #80
Conversation
added button lift on hover added extra shadow to buttons to help it stand out from the cards set the tracking on the buttons to be a bit wider as the text as upper case added background shadow to directionary text
✔️ Deploy Preview for helpafamily-margarita-humanitarian ready! 🔨 Explore the source changes: 31201b3 🔍 Inspect the deploy log: https://app.netlify.com/sites/helpafamily-margarita-humanitarian/deploys/611eb11c4d6f97000770d5c2 😎 Browse the preview: https://deploy-preview-80--helpafamily-margarita-humanitarian.netlify.app |
using margaritahumanitarian#79 fix to fix deploy errors
I like the hover effect, very clear interaction. |
I love what you did here @tof-tof! These sorts of subtle style and UX improvements to delight the visitor are wonderful. Thank you @RedFox0x20 for providing friendly guidance here. I appreciate you pitching in on the review side so much! Yep, the best time to pull commits from the I'm in meetings for awhile, but I'll follow up later with a more detailed technical review of your diff. |
@@ -1,4 +1,5 @@ | |||
import 'tailwindcss/tailwind.css'; | |||
// import 'tailwindcss/tailwind.css'; |
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.
// import 'tailwindcss/tailwind.css'; |
The diff looks good. I'm still getting the hang of Tailwind CSS and am a bit tired, so I'd actually like to revisit the rest of this review tomorrow with fresh eyes. @marekrozmus if you'd like, you're welcome to review and merge this before I return tomorrow. |
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.
@audreyfeldroy I'm not huge fan of lifting buttons as we are changing here default behaviour and look and feel of daisyui
but if you like it then go ahead 🙂
@@ -1,4 +1,5 @@ | |||
import 'tailwindcss/tailwind.css'; | |||
// import 'tailwindcss/tailwind.css'; | |||
import '../styles/global.css'; |
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.
Please place local imports below the external modules like:
import PropTypes from 'prop-types';
import React from 'react';
import '../styles/global.css';
.shaded-text{ | ||
@apply bg-black bg-opacity-10 py-2 rounded-lg; | ||
} | ||
|
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.
Please remove empty line
Thanks for the feedback about changing default daisyUI behavior @marekrozmus. It's a good point. Custom behavior does increase the maintenance effort on our side. I've given it serious consideration. My read of daisyUI is that it's like Bootstrap, where default components are provided but it's common/expected to customize the styles. https://daisyui.com/docs/customize looks like it encourages customization. Here the lift and shadow behavior adds enough improvement that I'd like to accept this PR. It's also subtle enough that I see it as compatible with #76. I do think it's the sort of change that could potentially be useful upstream to daisyUI. @tof-tof before I recommend that you submit an upstream PR: @saadeghi Hey, would you be interested in a daisyUI PR for the button lift and/or text shadow in this PR? See it in action in the Netlify deploy preview above. |
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.
Looks good to me.
@tof-tof Awesome PR! 👍🏻 |
@audreyfeldroy I encourage everyone to customize design decisions for their projects. Thanks to Tailwind's utility classes it's easy to customize things. |
added button lift on hover
added extra shadow to buttons to help it stand out from the cards
set the tracking on the buttons to be a bit wider as the text as upper case
added background shadow to directionary text
resolve #14