fix: Add visual invalid input handling beyond color#10101
Conversation
maribethb
left a comment
maribethb
left a comment
There was a problem hiding this comment.
adding a new media url is a breaking change, as discussed offline, but reviewed the rest of it
| } | ||
|
|
||
| /** Creates the DOM elements for the invalid input warning icon. */ | ||
| private createWarningIcon(parent?: Element | null): HTMLImageElement | null { |
There was a problem hiding this comment.
I'd go ahead and make this protected so that subclasses can modify it if desired
| if (!this.isTextValid_) { | ||
| dom.addClass(htmlInput, 'blocklyInvalidInput'); | ||
| aria.setState(htmlInput, aria.State.INVALID, true); | ||
| // insert the icon |
There was a problem hiding this comment.
I'd probably move this logic to a function call to keep the render_ method smaller and make it easier for subclasses to also call that function in case they override render.
| } | ||
|
|
||
| /** Creates the DOM elements for the invalid input warning icon. */ | ||
| private createWarningIcon(parent?: Element | null): HTMLImageElement | null { |
There was a problem hiding this comment.
Do you need to accept the parent as a parameter? you only call this in one place and you don't pass it in, you just append it manually to the dom after creation. I would remove this unless I'm overlooking something
There was a problem hiding this comment.
Good catch; this was leftover from a different approach I was taking earlier on. I'll remove it.
| import type {IFocusableNode} from './interfaces/i_focusable_node.js'; | ||
| import {Msg} from './msg.js'; | ||
| import * as renderManagement from './render_management.js'; | ||
| import {Svg} from './utils.js'; |
There was a problem hiding this comment.
i think you might want to have this import from ./utils/svg.js (similar to other imports from utils/ dir in this file) because sometimes importing from the whole utils.js file causes issues eventually
| Svg.PATH, | ||
| { | ||
| 'd': 'M2,15Q-1,15 0.5,12L6.5,1.7Q8,-1 9.5,1.7L15.5,12Q17,15 14,15z', | ||
| 'stroke': '#1f1f1f', |
There was a problem hiding this comment.
i'm somewhat iffy in my knowledge on styling svg with css, but i think if you move the stroke and fill properties to a css class it would let users customize the look of this easier if they want to. hardcoding it is really hard to override, i think. aaron may be more knowledgeable here.
There was a problem hiding this comment.
I believe you're correct. It's easy enough to move to css.
2 participants
The basics
The details
Resolves
Fixes #9684
Proposed Changes
Added a warning icon which is displayed when a field's input is invalid. This is to address the Use of Color WCAG guidelines.
Zelos Renderer
Zoomed in:
Zoomed out:
RTL:
Geras Renderer
Zoomed in:
Zoomed out:
RTL:
Test Coverage
Tests added to ensure icon is added appropriately and aria-invalid state is set.