ILIAS
ILIAS copied to clipboard
UI/Button: allow Glyphs as label
Allow Glyphs as labels for Buttons. In a second step, Actions will be removed from Glyphs.
This sharpens the separation and essential character of both components: Glyphs represent a concept, while Buttons trigger actions.
Hi @thibsy and @Amstutz,
this LGTM. Please have a look into this.
I'm not sure with the timeline here. The current proposal is to implement this in 9 and remove the old glyph-logic with ILIAS 10. The proposal is from January, though, and hence we might want to change this to 10/11. Any opinion?
Kind regards!
Hi @nhaagen
thx a lot for this PR. thx also for the deprecations.
- [x] First, concerning the targeted release. I would feel much more comfortable with only tackling 10/11, so please change the target to 10. IMO the deprecations could stay as is if the JF is fine with this.
Please answer the following Questions:
- [ ] Bulkies I: Check if there is a need to extend the description of the Button and/or Bulky to make the difference clear between the two (since containing a graphical element, is no differentiation anymore). If none is necessary, please shortly point out why you think so.
- [x] Bulkies II: Since they also get the getSymbol function, could they not lose the "getIconORGlyphMethod"? If so, note that also the implementation needs some reshuffling.
- [x] Context I: You add the Button context to the Symbol added. This looks redundant in the case of Bulkies if we align the getSymbol interface with the buttons. Is there an option to align the Button and special Bulkies implementation in the renderer somewhat?
- [ ] A11y: Currently there are A11y issues possible. E.g. if a Label is given along with a Glyph as a symbol with the same aria label (would be considered a redundant label). However, simply matching it to the bulky buttons implementation could cause an issue if no label is given at all (since the Glyph renderer of Bulkies does not render an aria label). This seems to be a bit of a can of worms. Maybe we should just always require a label to be set for buttons (and perhaps not always show it, e.g. if space is scarce)?
Please change:
- [x] getSymbol description: Wording is a bit odd. Rephrase to "Get the symbol as part of the Buttons's label if set." or similar, stressing there is only one to get, if one is set.
- [x] Rendering tests: Please add one with a none empty label and a symbol. Further, add at least one case, where the Button Label differs from the Glyphs aria and one where it is the same. This is important due to the different handling of arias.
- [x] Example Description for Testrail: Add the example descriptions for Testrail.
- [ ] Deprecation: Move the deprecation warning up to Glyph, so IDEs can hint on the problem. Basically, we'll deprecate that Glyph extends Clickable. Remove the deprecations from the implementation part of the interface.
- [x] getSymbol: Keep in internal
Since none of the points tackles the core of the proposal on the public interface, I will add the JF label, as soon as the branch is corrected.
Thx @Amstutz, @thibsy and @klees
Update: We will wait with the JF label untill we have a proposal on how to deal with the a11y point listed above.