Adds create dom for the category#4090
Adds create dom for the category#4090alschmiedt merged 2 commits intoRaspberryPiFoundation:toolboxfrom
Conversation
core/toolbox/category.js
Outdated
| */ | ||
| Blockly.ToolboxCategory.prototype.addColour_ = function(colour) { | ||
| if (colour) { | ||
| var border = '8px solid ' + (colour || '#ddd'); |
There was a problem hiding this comment.
Why is this hardcoded?
| this.htmlDiv_.appendChild(this.rowDiv_); | ||
|
|
||
| this.iconSpan_ = this.createIconSpan_(); | ||
| Blockly.utils.aria.setRole(this.iconSpan_, Blockly.utils.aria.Role.PRESENTATION); |
There was a problem hiding this comment.
Can you set roles inside your helper functions, at least for this, the label, and the group?
There was a problem hiding this comment.
I went back and forth with this a bit. By keeping these in the createDom method someone can overwrite the create methods without losing the accessibility labels. If we put them in their individual create methods we lose this, but increase the ability for the developer to create their own accessibility labels. Thoughts?
There was a problem hiding this comment.
Do we expect all changes to the create methods to end up needing the same accessibility labels?
I see your point, though. There's a balance between code structure and making sure that accessibility is the default, and I think the latter is more important here.
If the accessibility labels end up being more than a few lines, let's put them in helper methods going forward.
There was a problem hiding this comment.
Yeah, I think it depends a lot on the use case. They would definitely need to change the labels if they go away from the tree structure, but hard to say otherwise.
sgtm
2 participants
The basics
The details
Resolves
Adds create dom methods for a category.
Proposed Changes
Reason for Changes
Test Coverage
Tested on:
Documentation
Additional Information