#8878 closed enhancement (fixed)
[patch][cla] dijit.form.ComboBox would like ability to extend announce label
Reported by: | Nathan Toone | Owned by: | Nathan Toone |
---|---|---|---|
Priority: | high | Milestone: | 1.4 |
Component: | Dijit - Form | Version: | 1.3.0b3 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description (last modified by )
Sometimes, it is desirable to have a different announce label than the search terms. However, it is difficult to override this behavior in a subclass.
I suggest breaking out a protected function that will return the value to display for "announcements"
Attachments (1)
Change History (15)
Changed 12 years ago by
Attachment: | comboBoxAnnounce.patch added |
---|
comment:1 Changed 12 years ago by
Milestone: | tbd → 1.4 |
---|---|
Owner: | set to Nathan Toone |
comment:2 Changed 12 years ago by
Milestone: | 1.4 → 1.3.1 |
---|
Fixed in trunk in [17155] - changing milestone for consideration for inclusion in branch.
comment:3 Changed 12 years ago by
Milestone: | 1.3.1 → 1.4 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
comment:4 Changed 12 years ago by
Priority: | low → high |
---|---|
Resolution: | fixed |
severity: | minor → major |
Status: | closed → reopened |
toonetown, your change introduces a curious regression:
- Go to http://archive.dojotoolkit.org/nightly/checkout/dijit/tests/form/_autoComplete.html?testWidget=dijit.form.ComboBox
- Scroll down to "Custom labelFunc", which uses HTML labels
- Clear the input
- Arrow down to Alabama
- Observe the contents of the ComboBox? and listen to what the screen reader says
You will find with your change, that the ComboBox? now contains the following text:
<img width='97px' height='127px' src='images/Alabama.jpg'/>Alabama
This is the text for an image tag! Also, the screen reader makes no attempt to parse that markup, so it announces the markup itself: "i m g, width equals 97 p x, height equals ..."
You might make it so the ComboBox? only announces the label when it is plain text, but I'm concerned that's inconsistent with the HTML case. What do you think?
comment:5 Changed 12 years ago by
That's a very good point. What about adding in a different option - "announceAttr", and default it to the same value as searchAttr?
That gives the option for announcing something separate from the search or label attr...
In either case, I have rolled this patch back for now until we decide what to do - rolled back in [17187]
comment:6 Changed 12 years ago by
Hmm, I have a hard time imagining dijit users taking the time to learn what an announceAttr is. Could we just do innerText/textContent to get the plain text?
comment:7 Changed 12 years ago by
The potential problem with innerText/textContent is that it might create unexpected behavior. For instance:
Before<img width='97px' height='127px' src='images/Alabama.jpg'/>Alabama
The text content is "BeforeAlabama?" with no space in between. Yes it could be avoided, but it was unexpected considering a screen reader could parse that just fine.
Here's a thought: after we released 0.9, we got rid of a 'keyAttr' in FilteringSelect? for identifying the primary key of the data. How did we do this? We put it in the store as its "identifier":
{ identifier:"abbreviation", label: "name",
(just noticed there is also a "label" in there, shouldn't we get rid of labelAttr too?) So we could similarly add an optional "alt" or "title" attribute to the incoming data.
Unrelated: in the previous patch, the code was setting the newValue variable to the label text. That could be a problem since that same value gets passed to the autocomplete logic, where ComboBox? expects the searchAttr text in the box when it is autocompleting.
comment:8 Changed 12 years ago by
Ah sorry, I misunderstood what was going on. The screen reader discussion seems irrelevant, as the mere fact that the textbox gets the value
Before<img width='97px' height='127px' src='images/Alabama.jpg'/>Alabama
shows a problem.
As Mark said above, the value that gets put into the textbox is subsequently used by the autocomplete logic, being matched against searchAttr, so it seems to make sense to keep them the same.
Nathan, hy is it that you want the textbox to get a value different than the searchAttr? Was it something like capitalization? It seems like this might better be a job for DropDownSelect?
comment:9 Changed 12 years ago by
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
I am actually using FilteringSelect? - because I want it to be be filterable. But the announce part of it comes from ComboBox?. The problem happens when you have a labelAttr that is different than your searchAttr (which is completely valid).
searchAttr.) |
comment:10 Changed 12 years ago by
Description: | modified (diff) |
---|---|
Priority: | high → normal |
Resolution: | wontfix |
Status: | closed → reopened |
Summary: | [patch][cla] dijit.form.ComboBox announce option not honoring labelAttr, if set → [patch][cla] dijit.form.ComboBox would like ability to extend announce label |
I would like to reopen this bug - and make it a bit easier to extend this functionality in a subclass.
comment:11 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:12 Changed 12 years ago by
Type: | defect → enhancement |
---|
comment:13 Changed 12 years ago by
severity: | major → minor |
---|
comment:14 Changed 10 years ago by
Component: | Dijit → Dijit - Form |
---|
Read from labelAttr, if it exists. Fall back to searchAttr. This is what we do in other places. This can be committed post 1.3...