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

Always show URL bar. closes #1984#2038

Merged
bbondy merged 1 commit intobrave:masterfrom
cezaraugusto:feature-disable-title-mode
Jun 6, 2016
Merged

Always show URL bar. closes #1984#2038
bbondy merged 1 commit intobrave:masterfrom
cezaraugusto:feature-disable-title-mode

Conversation

@cezaraugusto
Copy link
Copy Markdown
Contributor

@cezaraugusto cezaraugusto commented Jun 2, 2016

diracdeltas
diracdeltas reviewed Jun 3, 2016
View reviewed changes
Comment thread app/extensions/brave/locales/en-US/preferences.properties Outdated
alwaysDeny=Always deny
appearanceSettings=Appearance settings
autoHideMenuBar=Hide the menu bar by default
disableTitleMode=Disable Title Mode
Copy link
Copy Markdown
Member

@diracdeltas diracdeltas Jun 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure people will know what 'Title Mode' is. Maybe change the text to 'Always show URL bar'?

Copy link
Copy Markdown
Contributor Author

@cezaraugusto cezaraugusto Jun 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diracdeltas I agree with you, haven't found a better name for it, so I just copied from the issue.

Should I change only i10n and keep everything else as disableTitleMode or change everything to showUrlBar instead?

Copy link
Copy Markdown
Member

@diracdeltas diracdeltas Jun 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fine to just change the l10n string

@diracdeltas
Copy link
Copy Markdown
Member

diracdeltas commented Jun 3, 2016

otherwise looks good to me. thanks!

@bbondy
Copy link
Copy Markdown
Member

bbondy commented Jun 4, 2016

Looking great!

Please remove the localization files other than English. They are generated only and downloaded from
Transifex. Then you can modify the commit with git rebase -i HEAD~2, or just squash it all together with git rebase -i master.

@bradleyrichter will be excited because he was just asking for this.

You can also set the default to: process.platform === 'win32' to avoid us having 3 titles on Windows until we can get rid of the titlebar. It is @bradleyrichter 's preference on Windows.

@bsclifton
Copy link
Copy Markdown
Member

bsclifton commented Jun 5, 2016 •
edited
Loading

Love this change (just pulled and tested it; Windows 10, x64). LGTM (pending @bbondy's feedback)! 😄

@bsclifton bsclifton changed the title disable title mode. closes #1984 Always show URL bar. closes #1984 Jun 5, 2016
@cezaraugusto
Copy link
Copy Markdown
Contributor Author

cezaraugusto commented Jun 6, 2016

hope it's all good now 👯

@bbondy
Copy link
Copy Markdown
Member

bbondy commented Jun 6, 2016

looks great, thank you!

@bbondy bbondy merged commit ac97957 into brave:master Jun 6, 2016
@luixxiul luixxiul added this to the 0.10.4dev milestone Jun 6, 2016
@cezaraugusto cezaraugusto deleted the feature-disable-title-mode branch July 3, 2016 06:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.10.4dev

Development

Successfully merging this pull request may close these issues.

5 participants

@cezaraugusto @diracdeltas @bbondy @bsclifton @luixxiul