-
Notifications
You must be signed in to change notification settings - Fork 63
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.
I love this idea! I'm wondering if there's a 'safer' way to implement it. For example, one idea would be a <TextLink>
component that wraps a <NuxtLink>
and applies the default text link styles.
Despite that, I think your approach works well because it ensures plain links are accessible against our standard text color by default 👍
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 like how much cleaner the class lists look now!
This approach might be a good way to get rid of the scss styles in the future, 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.
Instead of styling a
directly on the base, could we create a new Tailwind component like so
@layer components {
.text-link {
@apply text-pink hover:underline;
}
}
so that it only applies when we explicitly call for it to be applied?
In my experience, it's easier to apply classes when needed rather than unapply them when they're applied by default.
But yeah, I do appreciate that links are no-longer colored the same as regular text.
I precisely wanted to forget about repeating classes for normal links 😆 I pondered whether to create a new component or not but seemed not worth it for me. This is just a change in styles that certainly should be global. One can forget to use the customized link component, go directly with an Although it can be a good idea if we add the function to vary between an internal and external link if Openverse continues to exist in an iframe. If this change turns out to be more problematic in the future I'll happily create the component. |
* main: (73 commits) Make audio/image pages without ids show a 404 (#768) Fix logo button paddings and simplify implementation (#767) Fix global audio rtl close placement (#780) Check for `null` localStorage explicitly (#763) Truncate global audio text to two lines (#773) New image details page (#682) Switch to path-based i18n routing (#701) Enable source maps in production (#755) Remove Jamendo and Wikimedia Commons from audio meta sources (#747) Use jed1x json format to correctly handle pluralization (#753) Fix logo color on error page layout (#752) Add homepage content switcher for mobile screens (#716) Add inline-start border to filters on desktop (#748) Fix header items not fitting in (#718) Expose `close` to popover content via slot props (#736) Truncate row layout audio titles (#735) Stop blocking on analytics requests (#715) Style links globally (#727) Refactor the usage of i18n result count computation (#707) Use `VPopover` for the content report form (#719) ...
Description
This small change will save us from copying the styles for links several times in every place where one appears (I was doing this in the single result page for images). The default color is pink and they should have an underline on hover state.
Testing Instructions
Run the project, everything should look the same. Writing a new link should have the default styles without requiring to do anything else.
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin