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.
This looks good, thank you! It's tricky, the designs use a base font of 14px, but we can't set it globally because it seems to alter the entire page layout. Hopefully this is the last component that needs text-base
manually specified.
It looks great ✨ I noticed one spacing mistake on desktop, but not sure if it belongs to this work. The spacing between license description and between each license icon definition should be I would say that setting a bottom margin value instead of a top margin one solves the problem. |
5ad121a
to
65a8ac9
Compare
@@ -60,12 +60,12 @@ | |||
|
|||
<div class="mb-4 min-h-[7rem]"> | |||
<VDmcaNotice | |||
v-if="selectedReason === reasons.DMCA" | |||
v-if="media.foreign_landing_url && selectedReason === reasons.DMCA" |
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 fixing this! The warnings were bothering me. I guess they are fired before the 'image' object is loaded, when the image detail is pre-fetched.
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 great!
I have only noticed one small thing: the Report content button is charcoal-70 in the mockups, and is just charcoal currently. It is out of scope for this PR, though, so feel free to merge without changing.
Fixes
Fixes #1111 by @krysal
Description
This PR fixes the font size of the license description and breaks words in the attribution HTML tab to avoid causing horizontal scrolling when the page is on small screens.
Testing Instructions
Follow steps on issues and compare with Figma mockups.
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin