Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Improvements to about:passwords #13793

Closed
jonathansampson opened this issue Apr 11, 2018 · 7 comments
Closed

Improvements to about:passwords #13793

jonathansampson opened this issue Apr 11, 2018 · 7 comments

Comments

@jonathansampson
Copy link
Collaborator

Description

The page suggests users go to Preferences > Security to change how passwords are managed. Brave, however, doesn't have a Preference menu option. Instead, it should probably say Settings › Security.

Additionally, this should probably be a direct link to that resource, rather than directions.

Steps to Reproduce

Visit about:passwords in Brave.

Actual result:
Misleading, static text.

Expected result:
Hyperlink with accurate labeling.

Reproduces how often:
100%

Brave Version

0.22.13

about:brave info:

Brave: 0.22.13 
V8: 6.5.254.41 
rev: a8cfb160479f1d00d0769368eb440030182bb83b 
Muon: 5.1.2 
OS Release: 10.0.16299 
Update Channel: Release 
OS Architecture: x64 
OS Platform: Microsoft Windows 
Node.js: 7.9.0 
Brave Sync: v1.4.2 
libchromiumcontent: 65.0.3325.181

Reproducible on current live release:
Yes

@Blankbook2081
Copy link

about:passwords should also not show usernames/emails, and instead have a slider like this one

Just an idea.

@eljuno
Copy link
Contributor

eljuno commented Apr 11, 2018

I think MacOS use Preferences while Windows and Linux use Settings if I'm not wrong? And about:preferences said Preferences as a title.

@jevuu
Copy link

jevuu commented Apr 12, 2018

Yes, the directions are accurate for Mac, but not for Windows and Linux because the option reads "Settings" instead.

Seems like an easy fix, but what about localization? I think we'd have to create a new string (or more) for Windows and Linux?

For example:
passwordDisableInstructions=To change how passwords are managed, go to Preferences > Security
passwordDisableInstructionsWindowsLinux=To change how passwords are managed, go to Settings > Security

@jevuu
Copy link

jevuu commented Apr 13, 2018

How's this approach?
In passwords.js:

const isDarwin = require('../../app/common/lib/platformUtil').isDarwin()
...
      <div className={css(styles.passwordInstructions)}> 
        <span data-l10n-id='passwordDisableInstructions' />&nbsp;
        <span className={css(styles.passwordInstructionsLink)}
          data-l10n-id={isDarwin?'passwordInstructionsLinkDarwin':'passwordInstructionsLink'}
          onClick={aboutActions.createTabRequested.bind(null, {
            url: 'about:preferences#security'
        })} />
      </div>
...
const styles = StyleSheet.create({
  passwordInstructionsLink: {
    color: 'rgb(255, 80, 0)',
    textDecoration: 'underline',
    cursor: 'pointer',

    ':hover': {
      color: 'black',
    }
  },

passwords.properties:

passwordDisableInstructions=To change how passwords are managed, go to
passwordInstructionsLinkDarwin=Preferences > Security
passwordInstructionsLink=Settings > Security

Result (in Windows):
image

@jonathansampson
Copy link
Collaborator Author

jonathansampson commented Apr 14, 2018

@jevuu Looks pretty good; though I suspect we have a variable someplace that could replace the explicit rgb color value seen here:

color: 'rgb(255, 80, 0)',

@bsclifton Thoughts on this approach?

@jevuu
Copy link

jevuu commented Apr 19, 2018

Cool. I'll look through it and see if there is a variable for that.

Is it alright if I submit a pull request and continue the discussion there, or should I wait for a response?

jevuu added a commit to jevuu/browser-laptop that referenced this issue Apr 22, 2018
Fixes brave#13793 

* Changed password.js and passwords.properties
Modified strings for password.properties.
Added link in passwords.js to go to preferences#security

* Additional changes
passwords.properties and passwords.js
passwords.js - changed label to span, added &nbsp;
password.properties - removed space from end of string

* Change hard-coded color to variable.
@bsclifton
Copy link
Member

@jevuu you're always welcome to submit a PR (as you noticed 😄). Your PR was marked as work-in-progress... how's it going?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants