-
Notifications
You must be signed in to change notification settings - Fork 207
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
Add new feature notice for dark mode #4922
Conversation
600e5e1
to
b43865c
Compare
In accordance with #4936, the feature notice border has been changed to a single color instead of a gradient. The homepage color change should be a separate PR as that will involve some snapshot changes. |
b055198
to
90481bc
Compare
Full-stack documentation: https://docs.openverse.org/_preview/4922 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
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.
Sorry you had to spend so much time with the snapshots, but I'm glad that they did find a [small] problem with the language select size. Fixing it is the blocking request for change.
d30b1c1
to
3a2fc23
Compare
f037a41
to
1a6ace5
Compare
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.
Perfect ✨
Could you add a note in the Dark mode project to replace the link in the feature notice?
<template> | ||
<VLink | ||
:href="seeMoreHref" | ||
class="group flex h-10 w-full flex-row items-center gap-2 rounded-full border border-secondary pe-4 ps-2 hover:no-underline sm:w-[23.125rem]" |
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.
In general, I don't like to have a fixed width for an element that has text because it might overflow if a different language has longer text. But I tried Russian, which is usually longer, and it seems to fit. Hopefully, all other languages do, too.
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 did this because it was "Fixed" in Figma. When Francisco uses "Fixed" over "Hug", I usually see that as a sign that text is meant to wrap in those scenarios.
@@ -54,6 +54,8 @@ provide(ShowScrollButtonKey, showScrollButton) | |||
<style scoped> | |||
.app { | |||
grid-template-areas: "header" "main"; | |||
/* This is used by some elements. */ |
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.
Nice addition! It would be useful for the focus ring outlines.
Fixes
Fixes #4157 by @fcoveram
Description
This PR
VFeatureNotice
component that can be used for notifying people of new featuresVDarkModeFeatureNotice
that uses the above to notify people that we now have a dark modeVSelectField
based on theshowNewHighlight
boolean propTesting Instructions
Checklist
Update index.md
).main
) or a parent feature branch.ov just catalog/generate-docs
for catalogPRs) or the media properties generator (
ov just catalog/generate-docs media-props
for the catalog or
ov just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin