Improve link focus styles #89
Conversation
|
❌ Deploy Preview for koodikrapula failed. 🔨 Explore the source changes: 2b2c335 🔍 Inspect the deploy log: https://app.netlify.com/sites/koodikrapula/deploys/61780c70e88b070008ff5e0c |
src/js/utils/markdownIt.js
Outdated
| md.use(markdownItLinkAttributes, { | ||
| attrs: { | ||
| class: | ||
| 'underline text-red(hover:600 active:700) focus-visible:(outline-none ring-2 ring-blue-300 border-transparent)', |
There was a problem hiding this comment.
It might be good to export these classes from Link.js (e.g. as linkClasses) so the classes are not duplicated between two files (Link.js and here).
An alternative would be to add some preflight styles to twind.config.js, something like:
preflight: {
':focus-visible': 'outline-none ring-2 ring-blue-300',
}This style would apply to all focusable elements, not just links (<a>), so this could be better.
Btw we already have preflight styles for .prose a:hover and .prose a:focus (the .prose class comes from https://github.com/tw-in-js/typography), so no need to specify these classes in markdownIt.js: underline text-red(hover:600 active:700).
There was a problem hiding this comment.
Good idea 👍🏽 I left text-red classes to linkClasses const so it can be used anywhere. No need to add text-red classes to Link component seperately now.
There was a problem hiding this comment.
Hmm, good idea. 👍 These might now be unnecessary in twind.config.js (npm start must be restarted after modifying this file):
'.prose a:hover': apply('text-red-600'),
'.prose a:active': apply('text-red-700'),Remove these if you want extra points. 😀
| @@ -40,6 +40,8 @@ module.exports = { | |||
| // because sometimes it's clearer than template strings | |||
| 'prefer-template': 'off', | |||
|
|
|||
| 'no-unused-vars': 'warn', | |||
There was a problem hiding this comment.
Nit: move this before 'no-use-before-define' so that the built-in rules are sorted alphabetically. 😂
| "preact": "^10.5.13", | ||
| "preact-render-to-string": "^5.1.19", | ||
| "prettier": "2.3.2", | ||
| "react": "^17.0.2", | ||
| "twind": "^0.16.16" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/eslint": "^7.2.13" | ||
| "@types/eslint": "^7.2.13", | ||
| "eslint-plugin-simple-import-sort": "^7.0.0" |
There was a problem hiding this comment.
ESLint is run in Netlify deploy preview builds, and Netlify installs only dependencies (not devDependencies), so this must be moved back to dependencies. The build is currently failing because of this.
| @@ -1,6 +1,7 @@ | |||
| import { ExternalLinkIcon } from '@heroicons/react/solid' | |||
| import { html } from 'htm/preact' | |||
| import { apply, tw } from 'twind' | |||
| import { linkClasses } from '../utils/const' | |||
There was a problem hiding this comment.
| import { linkClasses } from '../utils/const' | |
| export const linkClasses = | |
| 'underline text-red(hover:600 active:700) focus-visible:(outline-none ring(2 blue-300 offset-2))' |
I would move this here because the classes are closely related to the Link component. Wdyt?
See also Locality of Behavior / Co-location. 😜
Fixes #70