browser: accessibility: add label to combobox #9905
Conversation
| Labels : { | ||
| '.uno:CharFontName' : _('Font:'), | ||
| '.uno:FontHeight' : _('Size:'), | ||
| '.uno:StyleApply' : _('Style:') |
There was a problem hiding this comment.
we usually try to avoid :
| @@ -3,13 +3,18 @@ | |||
| * L.Map.StateChanges stores the state changes commands coming from core | |||
| * LOK_CALLBACK_STATE_CHANGED callback | |||
| */ | |||
| /* global $ app */ | |||
| /* global $ app _ */ | |||
| /*eslint no-extend-native:0*/ | |||
| L.Map.mergeOptions({ | |||
| stateChangeHandler: true | |||
| }); | |||
|
|
|||
| L.Map.StateChangeHandler = L.Handler.extend({ | |||
There was a problem hiding this comment.
StateChangeHandler seems to be not a perfect place for such things.
Let's keep it doing only one thing - storing status values for a command.
We use _UNO('command') for getting the translated "label" for uno command, which seems to already exist and do that correctly :) can we reuse it or there is some problem?
There was a problem hiding this comment.
Let me check, I did read it. It translate with hotkeys underscore I think.
| @@ -183,6 +183,12 @@ JSDialog.combobox = function (parentContainer, data, builder) { | |||
| var container = L.DomUtil.create('div', 'ui-combobox ' + builder.options.cssClass, parentContainer); | |||
| container.id = data.id; | |||
|
|
|||
| var labelText = L.Map.StateChangeHandler.prototype.Labels[data.command]; | |||
There was a problem hiding this comment.
in general - JSDialog widgets should be built only based on data available inside JSON.
For correct behavior on updates, etc. We need "widget building function" to be stateless (not depending on external state).
There was a problem hiding this comment.
Occurs that the stateless does not have label info data, so If you read the task description,
I am doing a short path (hack) to resolve the problem.
| var labelText = L.Map.StateChangeHandler.prototype.Labels[data.command]; | ||
| if (labelText) { | ||
| var label = L.DomUtil.create('label', '', container); | ||
| label.textContent = labelText; |
There was a problem hiding this comment.
Why do we want that? combobox is the combobox field (input + dropdown).
If we want some label, we should put separate label widget next to it (in the .ui file in core, in the JSON).
There was a problem hiding this comment.
I did it, but for some reason, the parent container is removed and it messes up the combo box.
Somewhere it is not using well the combox which it is out of the scope of the task.
On the other hand, the label requires the LO Core .ui file changes that I am trying to avoid to do it.
There was a problem hiding this comment.
it's acctually what we should do (addin in .ui file).
Container can be replaced when we do some updates to the widget.
see update or action commands handling executeAction or updateWidget in JSDialogBuilder.
they work correctly as I know, but if you changed the structure here... action might not be able to change eg. text
There was a problem hiding this comment.
Ok, Please clarify in the task description that the changes also require changes in LibreOffice desktop accessibility.
hcvcastro
force-pushed
the
pr/master/7A
branch
5 times, most recently
from
b286fc6
to
03bae19
Compare
timar
force-pushed
the
master
branch
2 times, most recently
from
ebbad3c
to
61cf2b4
Compare
hcvcastro
force-pushed
the
pr/master/7A
branch
3 times, most recently
from
e428973
to
af6b5a2
Compare
. Change-Id: Ib462aa34f450a84ad27c1a37ed554dff772c1c27 Signed-off-by: Henry Castro <[email protected]>
Change-Id: Ib462aa34f450a84ad27c1a37ed554dff772c1c27
Signed-off-by: Henry Castro [email protected]