Page MenuHomePhabricator

Don't show error and CTA boxes together on login/accountcreate forms when in an error state
Closed, ResolvedPublic2 Story Points

Description

Go to the login form and try to login with a bad user/password combo.

Expected: The gray box shouldn't show and the error box should.
Actual: Both are showing

This didn't happen before. When designing we purposely wanted to only show the CTA to people at the beginning of the workflow. We should add a test and fix this.

AC

  • When the user navigates to the login form, they should see the CTA.
  • Hide welcome message for all failed POST requests on login form
  • should work the same on createaccount form

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 28 2016, 10:02 AM
ovasileva triaged this task as High priority.Nov 1 2016, 2:58 PM
ovasileva lowered the priority of this task from High to Normal.Nov 2 2016, 7:49 PM
ovasileva moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.
Jdlrobson added a subscriber: Nirzar.

@Nirzar thoughts? Is the description as written correct?

Jdlrobson assigned this task to Nirzar.Apr 20 2017, 7:10 PM
Nirzar added a comment.EditedJun 27 2017, 11:38 PM

Yes this is correct. Show only the error message

Bonus:
We did lot of work around input field state management. Error states was a big part of it. Idk if MediaWiki UI has error states in it. in OOJS UI we turn the border of the error field red.

If we can actually show where the error is, that helps the user enormously

Jdlrobson moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.
Jdlrobson added a subscriber: Volker_E.

If we can actually show where the error is, that helps enormously for the user.

Agreed. I'm not sure if this is on @Volker_E radar but we'd need to OOjs-ui-ify the login form... tracked in T85853 ?

sound good!

phuedx renamed this task from We show error and cta boxes together on login form to Don't show error and CTA boxes together on login form when in an error state.Jul 5 2017, 10:51 AM
phuedx updated the task description. (Show Details)
phuedx updated the task description. (Show Details)
ovasileva set the point value for this task to 2.Jul 5 2017, 4:15 PM
pmiazga claimed this task.Jul 14 2017, 6:11 PM

Change 365871 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/skins/MinervaNeue@master] Do not show warning box when LoginForm was posted

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

pmiazga added a comment.EditedJul 17 2017, 11:03 PM

As it's pretty difficult to detect failed login form I decided to not show the message when a user sends POST request. This should do the trick valid POST request will redirect to a different page, only bad POST request will cause two boxes to be visible.

@Nirzar - Special:CreateAccount page had same logic (same warning box was always visible). I decided to be consistent and do same behavior both on LoginPage and CreateAccount. When user form is invalid we will present only form error without gray box. If you agree with it please update task description to include both Login and CreateAccount pages.

pmiazga edited projects, added MinervaNeue; removed MobileFrontend.
Nirzar updated the task description. (Show Details)Jul 17 2017, 11:05 PM

@pmiazga works for me. desc updated

pmiazga renamed this task from Don't show error and CTA boxes together on login form when in an error state to Don't show error and CTA boxes together on login/accountcreate forms when in an error state.Jul 18 2017, 3:20 PM
pmiazga updated the task description. (Show Details)
Jdlrobson moved this task from Backlog to Bugs on the MinervaNeue board.Jul 18 2017, 6:45 PM

Change 365871 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Do not show warning box when LoginForm was posted

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

ovasileva reassigned this task from pmiazga to ABorbaWMF.Jul 20 2017, 12:41 PM
ovasileva added subscribers: ABorbaWMF, pmiazga, ovasileva.

over to you @ABorbaWMF! testing criteria is in the task decription

Tested on the beta cluster on android and ios devices. Everything looks good.

phuedx reassigned this task from ABorbaWMF to ovasileva.Jul 24 2017, 12:58 PM
phuedx added a subscriber: phuedx.
ovasileva closed this task as Resolved.Jul 24 2017, 4:00 PM