Page MenuHomePhabricator

Add "You are not logged in" overlay popup on termbox editing
Closed, ResolvedPublic8 Story Points

Description

As as non-logged in editor
When I click the edit button
I will be informed on how my edits will be attributed (via IP address or user name)
so that I can participate in the open data movement with confidence

Mock:

Acceptance Criteria

  • When an anonymous user clicks on the edit icon, display a pop up
    • The pop up contains what is shown in the mock, with the exception of the "remember my choice option" (this is covered in T221833)
    • have a layer effect on the editable page (e.g. transparent black overlay as currently seen in the mocks)
    • direct to the log in page (in the same tab) when clicking the log in button, and redirect to the edit page once the user logged in
    • direct to the sign up page (in the same tab) , when clicking the sign up button, and redirect to the edit page once the user signed up
  • allow editing with the IP adress when clicking on the CTA (Call to Action button)
  • follow the design specified in the Figma file (yet to complete)

Remarks:

  • Log in popup cannot be "clicked away" without making a decision
  • This story only concerns the mobile termbox. For the desktop behaviour refer to T221779.
  • during task break-down we decided to not prevent edit by no means other than adding an overlay on top of the text fields, i.e. edits that are indeed performed by circumventing it will be treated like they chose "edit without logging in"
  • no little "collapse overlay" arrow will be added

Event Timeline

WMDE-leszek set the point value for this task to 8.
WMDE-leszek updated the task description. (Show Details)Apr 25 2019, 8:48 AM
Lea_WMDE updated the task description. (Show Details)May 6 2019, 10:59 AM
Lea_WMDE triaged this task as Normal priority.May 6 2019, 11:04 AM
Lea_WMDE updated the task description. (Show Details)
Tarrow renamed this task from Add "Your are not logged in" overlay popup on termbox editing to Add "You are not logged in" overlay popup on termbox editing.May 8 2019, 8:49 AM

Due to some miscommunication we started this ticket without the most current version of mocks :/ It should actually look like shown below. Is this a small enough change that we can still put this in this ticket, or would you want to reestimate/ do a follow up /first discuss it, because this has pit falls the current version does not?


(Please ignore the pink box, that's my ignorance when it comes to figma, and since we changed the text of the checkbox today I think the spacing went a bit off as well)

Tarrow added a comment.May 9 2019, 2:35 PM

From my side this is fine to leave. If the complexity were to change I would expect it to be encapsulated in T221833

Hi Jakob. Thanks a lot for the link. It mostly looks great. I have a few picky remarks.

  1. Could you change the font weight in the buttons and the headline from 700 to 500
  2. Could youu change the line heights from 1 to 1.2
  3. Could you change the font weight of the message text from 400 to 300

Thank you!

Hi @Hanna_Petruschat_WMDE,

I looked into your change requests - thanks for the very precise instructions. There are a few questions, though.

1 Could you change the font weight in the buttons and the headline from 700 to 500

There are two challenges
a) For the button this diverges from OOUI (where "bold" = 700 is in use). Is this intended? Should this change apply for our button component in general or is this mean exclusively for the buttons contained in the "You are not logged in" overlay?
b) font-weight support for different fonts is a delicate subject. Depending on the device and the available fonts some numeric weights do share the same actual font used. In my case e.g. all values from 100 to 500 result in the same visible weight, consequently for me buttons & headline would then look like "normal text".

  1. Could you change the line heights from 1 to 1.2

Will do

  1. Could you change the font weight of the message text from 400 to 300

Again, this change effectively causes no visual change on my device.

Depending on the fonts installed the very specific numeric values may make sense or not. This is not to say that this change does not bring value to may devices, just wanted to bring it to your attention so it is a conscious decision. FWIW these are the weights (basically by the book) that famous bootstrap uses.

Change 511924 had a related patch set uploaded (by Pablo Grass (WMDE); owner: Pablo Grass (WMDE)):
[wikibase/termbox@master] AnonEditWarning: mend styling

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

Hi @Hanna_Petruschat_WMDE,

I'm not quite sure what to do about this. I created a patch to reflect the changes requested but am uncertain if I should hand it in to the other devs to review or if there will be more input about the questions stated above.

Thanks

Hi @Pablo-WMDE , sorry for the late reply. Also: I cannot really get this discussion to a farther point I asssume. If I look at the initial fonts on my machine, they appear all way heavier than the ones represented in the OOUI demos. I played around with the font weight to get to the numbers mentioned above. So maybe it's okay to not see a difference on your device?

As yesterday here the short summery of how we go on with the font-size question:
OOUI defines the default font-size of 0.875em (===14px)[1]. The only exception is for screen-width-sizes of 320px-960px[2], there the font-size is 1em. After checking out that we need 1em as default font-size for mobile and it does not make sense to decrease font size on wider screens. So UX decided to keep our current state of 1em(=== 16px ) font-size in Termbox.

  1. see: https://phabricator.wikimedia.org/diffusion/GOJU/browse/master/src/themes/wikimediaui/common.less$4-5
  2. play with width of https://doc.wikimedia.org/oojs-ui/master/demos/?page=dialogs&theme=wikimediaui&direction=ltr&platform=mobile#demo-section-buttons

Change 511924 merged by jenkins-bot:
[wikibase/termbox@master] AnonEditWarning: mend styling

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

Change 514009 had a related patch set uploaded (by Jakob; owner: Jakob):
[mediawiki/extensions/Wikibase@master] Update Termbox

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

Change 514009 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Update Termbox

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

This is on beta with all sub tasks now.
Of course, the checkbox (T221833) is not there yet.

/cc @Lea_WMDE

I checked and there is a few things that I found, but I don't expect any of this to be an acceptance blocker:

  • If I log in after having clicked on "Log in" on our overlay, when they redirect me to the page I receive the info that I am logged in, but need to reload:


This info does not go away, even after hard reloads, and our overlay also doesn't disappear. I am assuming that this is a beta wiki issue, and the fact that the overlay still shows up is related to the wiki, although knowing that I am logged in, not being able to deal with that fact. Do you concur?

  • Right now, I think the buttons will be in a separate line each for most of the phones. But I will follow up with @Hanna_Petruschat_WMDE and if necessary, this would become a new ticket.

@Lea_WMDE It would be interesting to see under which circumstances this (the "Central logi" overlay) happens for you - I can't really reproduce this. Even if it does not sound like it could be our fault it should still be reproducible.

FWIW the buttons wrapping to multiple lines under certain conditions is a known discrepancy from the mocks, but the way they wrap was explicitly handled in coordination with @Hanna_Petruschat_WMDE. We should have done a better job documenting the decision.

Regarding the notice: When logging in I got redirected to the desktop version and needed to switch back to mobile. When doing so, I also got the the notice. I could click it away but got it back when reloading as it stated. @Pablo-WMDE if you like, you can test and reproduce on my machine.

Pablo-WMDE added a comment.EditedJun 14 2019, 9:12 AM

@Hanna_Petruschat_WMDE was kind enough to showcase this to me, click path is a follows

  1. open mobile item page (trough useformat=mobile query not m. subdomain), e.g. https://wikidata.beta.wmflabs.org/w/index.php?title=Q11&useformat=mobile
  2. click "edit pen"
  3. click "log in" in the overlay
  4. login page in desktop layout opens
  5. logging in with correct credentials
  6. item page (same item as before) in desktop layout opens
  7. scroll to bottom, click the "Mobile view" link
  8. item page (same item as before) in mobile layout opens (trough m. subdomain)
  9. warning is shown "Central login - You are centrally logged in. Reload the page to apply your user settings."
    • the page behaves as if not logged in
    • reloading does not resolve this - message re-appears, page behaves as not logged in

Interestingly the same behavior can be observed when, in step two, clicking the "watchlist star" instead of the "edit pen" - i.e. it is not limited to/caused by termbox.

The click path can be abbreviated to

  1. open the desktop home page, e.g. https://wikidata.beta.wmflabs.org/wiki/Wikidata:Main_Page
  2. click "log in" in the top nav
  3. login page in desktop layout opens
  4. logging in with correct credentials
  5. home page in desktop layout opens
  6. scroll to bottom, click the "Mobile view" link
  7. home page in mobile layout opens (trough m. subdomain)
  8. warning is shown "Central login - You are centrally logged in. Reload the page to apply your user settings."
    • the page behaves as if not logged in
    • reloading does not resolve this - message re-appears, page behaves as not logged in

@Pablo-WMDE thanks for documenting this! I was going a bit further with it, trying to create a new account as well. Here is what happened:

  1. click "edit pen"
  2. click "Create an Account" in the overlay
  3. create an account page in desktop version opens
  4. created an account
  5. get redirected to the item page I came from (now also in desktop version)
  6. switch to mobile by clicking the "mobile version" link
  7. checking, if I'm still logged in (I'm not)

Then the circle of the above begins: clicking the edit pen, click "log in" in the overlay, get directed to the desktop log in page, but instantly automagically logged in in the background and redirected to the desktop item page (without any action of mine).

Regarding the version history:
When on mobile view,
clicking the "edit pen",
choosing "editing without log in" in the overlay,
then I edit and publish,
seeing the item in mobile version with my edit.
When switching to the desktop version (by hitting the link),
I'm logged in without anything happening in between,
the version history shows my IP address.

While researching this we filed T225814.
I would be happy if we could close this story despite this (unrelated) incident. Verification of the AC of this story should be possible by logging in through the login page of the mobile beta site.

/cc @Lea_WMDE @Hanna_Petruschat_WMDE

Hanna_Petruschat_WMDE closed this task as Resolved.Jun 17 2019, 7:47 AM
Hanna_Petruschat_WMDE claimed this task.

Sorry for the long wait but finally \o/