-
Notifications
You must be signed in to change notification settings - Fork 63
changed the scroll to top button color B->P #1036
Conversation
I am not sure how to add labels to a pull request and how to eliminate the errors can someone please help me out with this |
thanks @sarayourfriend |
There are some end to end errors, not sure due to the bg color change or some other issue-
Can some one pls suggest some changes I could make ? |
src/components/VScrollButton.vue
Outdated
@@ -2,7 +2,7 @@ | |||
<button | |||
:aria-label="$t('browse-page.aria.scroll')" | |||
type="button" | |||
class="scroll text-white bg-trans-blue hover:bg-trans-blue-action transition-all duration-100 ease-linear fixed bottom-4 w-14 h-14 hover:shadow-md rounded-full text-center" | |||
class="scroll text-white bg-trans-pink hover:bg-trans-pink-action transition-all duration-100 ease-linear fixed bottom-4 w-14 h-14 hover:shadow-md rounded-full text-center" |
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.
Could you change the colors:
: bg-trans-pink
bg-pink
: hover:bg-trans-pink-action
bg-dark-pink
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.
Thank you for your PR, @kb-0311 ! We are using Tailwind, and only some pre-configured colors are allowed in the class names. Here are the definitions of the pink that the button should use:
openverse-frontend/tailwind.config.js
Lines 27 to 31 in e23af1a
// Brand | |
yellow: '#ffe033', | |
pink: '#c52b9b', | |
// Active | |
'dark-pink': '#7c2264', |
There are no such colors as
bg-trans-blue
, that's why you are seeing the errors.
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.
ohhh got it thanks also I made the changes you requested.
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.
@obulat actually I am pretty sure I added the dark class instead of the trans class however still giving some E2E error.
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.
Great! Your changes are correct, and the e2e failure is not related to them. Let me look into that.
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.
ok thanks for your feedback hoping to contribute more to the project in the future
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 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.
Thank you!
Fixes
Fixes #1032 by @zackkrida
Description
Changed the scroll to the top button component color from blue to pink
Testing Instructions
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin