widelands

Merge lp://staging/~widelands-dev/widelands/bug-1827786-metaserver-login-box into lp://staging/widelands

  1. bug-1827786-metaserver-login-box
  2. Merge into trunk
Proposed by Toni Förster
Status: Superseded
Proposed branch: lp://staging/~widelands-dev/widelands/bug-1827786-metaserver-login-box
Merge into: lp://staging/widelands
Prerequisite: lp://staging/~widelands-dev/widelands/bug-1825932-open-games
Diff against target: 188 lines (+104/-2) (has conflicts)
3 files modified
src/ui_fsmenu/multiplayer.cc (+12/-0)
src/wui/login_box.cc (+81/-2)
src/wui/login_box.h (+11/-0)
Text conflict in src/ui_fsmenu/multiplayer.cc
Text conflict in src/wui/login_box.cc
Text conflict in src/wui/login_box.h
To merge this branch: bzr merge lp://staging/~widelands-dev/widelands/bug-1827786-metaserver-login-box
Reviewer Review Type Date Requested Status
kaputtnik Pending
Review via email: mp+367100@code.staging.launchpad.net

This proposal has been superseded by a proposal from 2019-05-12.

Commit message

redesigned login box

- limit the possible characters for usernames
- draw a red box around the input field for erroneous input
- tell user were to register their username
- clicking registered checkbox focuses password field
- remove check from registered clears password field
- password field is only accessible when checkbox is clicked
- when a password is set, ***** is shown onopening

multiplayer login redesign

- only show login dialog when no name is set
- always show login box settings button

To post a comment you must log in.
Revision history for this message
kaputtnik (franku) wrote :

It would be good to have the registered checkbox above the password field. It would be a more logical ordering, imho.

Revision history for this message
Toni Förster (stonerl) wrote :

Your're right. Chnaged it.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4915. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/529968856.
Appveyor build 4696. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1827786_metaserver_login_box-4696.

Revision history for this message
GunChleoc (gunchleoc) :
Revision history for this message
Toni Förster (stonerl) wrote :

Will address the strings later. For the others see the diff comments.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4925. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/530433164.
Appveyor build 4706. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1827786_metaserver_login_box-4706.

Revision history for this message
kaputtnik (franku) wrote :

As talked on IRC: Can you try to remove the red border around the password editbox and instead show the string 'Password' and the corresponding editbox look sort of grayed out?

I think the text below can also be improved: "You need an account on the widelands website to use a registered account. Please visit ...."

Revision history for this message
GunChleoc (gunchleoc) wrote :

Changing editbox styles is implemented in https://code.launchpad.net/~widelands-dev/widelands/font_size-lua/+merge/366938, so we really need somebody to review that branch and make Toni's life easier.

Revision history for this message
GunChleoc (gunchleoc) wrote :

The changes string is missing a . after the %

I'll do some testing before I reply to the remaining diff comments.

Revision history for this message
Toni Förster (stonerl) wrote :

@kaputtnik

I addressed your comments. The only thing I can't do ATM is, greying out the password field.

Revision history for this message
Toni Förster (stonerl) wrote :

But I'm not confident whether we should grey the password field out as well. I would look too similar to an active edit box. Greying out the text "Password" and making the input box not editable should be sufficient, IMHO.

Revision history for this message
Toni Förster (stonerl) wrote :

This one gets replaced by the following one:

https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367312

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: