Page MenuHomePhabricator

Focusing a template field or adding a new field focusses the info icon, not the text widget
Closed, ResolvedPublic8 Story Points

Description

This regression was caused by the booklet layout focus code we changed.

Details

Event Timeline

Esanders created this task.Oct 3 2015, 11:29 AM
Esanders raised the priority of this task from to Needs Triage.
Esanders updated the task description. (Show Details)
Esanders added a subscriber: Esanders.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 3 2015, 11:29 AM

Worse still, clicking on a the text input box causes the focus to go the (i) until the second time you click it :/

Esanders triaged this task as High priority.Oct 3 2015, 11:38 AM
Esanders set Security to None.
Esanders updated the task description. (Show Details)Oct 3 2015, 12:36 PM
Esanders added a project: OOUI.
Esanders added a subscriber: matmarex.

Change 243387 had a related patch set uploaded (by Esanders):
BookletLayout: Fix focus of page switching

https://gerrit.wikimedia.org/r/243387

This is a pretty serious UX regression so I'd vote for a backport.

Esanders renamed this task from Adding a field to a template focusses the info icon, not the text widget to Focusing a template field or adding a new field focusses the info icon, not the text widget.Oct 3 2015, 1:18 PM
Esanders claimed this task.

Change 243388 had a related patch set uploaded (by Esanders):
MWParameterPage: Focus value input when focussing page

https://gerrit.wikimedia.org/r/243388

Wouldn't a better fix for this be to change the DOM order of the buttons and text field in MWParameterPage? It would also remove the need for manual tab indices.

Change 243527 had a related patch set uploaded (by Bartosz Dziewoński):
ve.ui.MWParameterPage: Use more natural DOM order for text field and buttons

https://gerrit.wikimedia.org/r/243527

https://gerrit.wikimedia.org/r/243387 is a good change to make anyway (especially the first part, "Use document.activeElement to check if the page has focus as :focus doesn't work until a later event cycle"), but I think https://gerrit.wikimedia.org/r/243527 is a better fix than https://gerrit.wikimedia.org/r/243388 (and it doesn't have https://gerrit.wikimedia.org/r/243387 as a dependency). Either should be fine though. Somebody please decide and merge one of them :)

Merged DOM order fix, but I think we should merge https://gerrit.wikimedia.org/r/243388 anyway. Explicitly stating which input to focus is faster than calculating which is first, and will may prevent against regressions.

Change 243527 merged by jenkins-bot:
ve.ui.MWParameterPage: Use more natural DOM order for text field and buttons

https://gerrit.wikimedia.org/r/243527

Change 243387 merged by jenkins-bot:
Follow-up I4acbe69420: BookletLayout: Fix focus of page switching

https://gerrit.wikimedia.org/r/243387

Change 243704 had a related patch set uploaded (by Jforrester):
ve.ui.MWParameterPage: Use more natural DOM order for text field and buttons

https://gerrit.wikimedia.org/r/243704

Jdforrester-WMF closed this task as Resolved.Oct 5 2015, 6:01 PM
Jdforrester-WMF removed a project: Patch-For-Review.
Jdforrester-WMF edited a custom field.

Change 243704 merged by jenkins-bot:
ve.ui.MWParameterPage: Use more natural DOM order for text field and buttons

https://gerrit.wikimedia.org/r/243704

Change 243388 merged by jenkins-bot:
MWParameterPage: Focus value input when focussing page

https://gerrit.wikimedia.org/r/243388

Jdforrester-WMF moved this task from Backlog to Reviewing on the OOUI board.Nov 21 2015, 2:30 AM