chore: add alt tag #297
chore: add alt tag #297
Conversation
e11sy
left a comment
e11sy
left a comment
There was a problem hiding this comment.
alt `contenteditable` should have same spacing as a caption
| <div id="editorjs"></div> | ||
| <script src="https://cdn.jsdelivr.net/npm/@editorjs/editorjs@latest"></script> | ||
| <script type="module"> | ||
| import ImageTool from "../src/index.ts"; |
There was a problem hiding this comment.
unexpected change, dev mode should not depend on built files
src/index.ts
Outdated
| /** | ||
| * Alt text enabled state | ||
| * Null when user has not toggled the alt tune | ||
| * True when user has toggled the alt tune | ||
| * False when user has toggled the alt tune | ||
| */ | ||
| private isAltEnabled: boolean | null = null; |
There was a problem hiding this comment.
why we need to distinguish when user has not toggled the alt tune and when user has toggled the alt tune to false?
maybe we can initialise it with false?
|
Is this good? @e11sy |
| &__alt { | ||
| visibility: hidden; | ||
| position: absolute; | ||
| bottom: 0; | ||
| left: 0; | ||
| margin-bottom: 10px; | ||
|
|
||
| &[contentEditable="true"][data-placeholder]::before { | ||
| position: absolute !important; | ||
| content: attr(data-placeholder); | ||
| color: #707684; | ||
| font-weight: normal; | ||
| display: none; | ||
| } | ||
|
|
||
| &[contentEditable="true"][data-placeholder]:empty { | ||
| &::before { | ||
| display: block; | ||
| } | ||
|
|
||
| &:focus::before { | ||
| display: none; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
can we reuse existing input styles and do not duplicate them?
src/index.ts
Outdated
| tunes.push({ | ||
| name: 'alt', | ||
| icon: IconText, | ||
| title: 'With alt text', |
There was a problem hiding this comment.
should it be applied to caption as well?
src/ui.ts
Outdated
|
|
||
| // Add alt attribute for IMG tags | ||
| if (tag === 'IMG') { | ||
| attributes.alt = this.nodes.alt.innerHTML || ''; |
There was a problem hiding this comment.
I'd suggest to get textContent instead
src/index.ts
Outdated
| name: this.api.i18n.t('caption'), | ||
| icon: IconText, | ||
| title: this.api.i18n.t('With caption'), | ||
| toggle: true, | ||
| }); | ||
| } | ||
|
|
||
| if (this.config.features?.alt === 'optional') { | ||
| tunes.push({ | ||
| name: this.api.i18n.t('alt'), |
There was a problem hiding this comment.
i18n is not needed for name because it is used only for key name in result JSON, not for the UI.
Leave it for title only.
3 participants
Problem
This PR resolves #30 by adding support for alt text in the Image Tool. Currently, the Image Tool lacks the ability to add alternative text to images, which is essential for accessibility (WCAG compliance) and improves SEO.
Cause
The Image Tool was missing:
Solution
altfield to ImageToolData interfacealtconfiguration option to FeaturesConfigConfiguration
The alt text feature can be configured via: