Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sentence casing on Always Show Full URLs in brave://settings/appearance #16939

Closed
stephendonner opened this issue Jul 14, 2021 · 16 comments · Fixed by brave/brave-core#9461
Closed
Assignees
Labels
needs-text-change This change requires some careful wording. OS/Desktop OS/macOS polish Nice to have — usually related to front-end/visual tasks priority/P4 Planned work. We expect to get to it "soon". QA Pass-macOS QA/Yes release-notes/exclude

Comments

@stephendonner
Copy link

Description

Steps to Reproduce

  1. load brave://settings/appearance
  2. look at the following two entries:
    Always show bookmarks on new tab page
    Always Show Full URLs

Actual result:

Always Show Full URLs is in all uppercase/Initial Caps.

example example
Screen Shot 2021-07-13 at 6 00 28 PM Screen Shot 2021-07-13 at 6 02 08 PM

Expected result:

Always show full URLs (lowercase, to match the above setting description)

Reproduces how often:

100%

Brave version (brave://version info)

Brave 1.28.77 Chromium: 92.0.4515.93 (Official Build) nightly (x86_64)
Revision 6eb43ff7850a1d710c3f827a0555737c74edab5c-refs/branch-heads/4515@{#1378}
OS macOS Version 11.4 (Build 20F71)

/cc @rmcfadden3

@stephendonner stephendonner added good first issue needs-text-change This change requires some careful wording. OS/Desktop labels Jul 14, 2021
@rebron
Copy link
Collaborator

rebron commented Jul 15, 2021

cc: @darkdh :-)

@rebron rebron added priority/P4 Planned work. We expect to get to it "soon". polish Nice to have — usually related to front-end/visual tasks labels Jul 15, 2021
@swarup4741
Copy link

Can i work on this issue ?

@stephendonner
Copy link
Author

Can i work on this issue ?

Hi @swarup4741 - absolutely! Thanks, I'll assign this to you. If for some reason you're unable to pursue it further, please un-assign yourself so others can pick it up.

A good place to start is to look through https://github.com/brave/brave-core. Cheers!

@swarup4741
Copy link

@stephendonner is it only a issue on macOS .. cause I'm using windows 10 and i am having it correctly said...

brave

@swarup4741
Copy link

I was searching a lot of directory but i found this one close (brave-core/browser/resources/settings/brave_appearance_page/) but exactly couldn't figure out where to make changes

@stephendonner if you could please guide me a little bit

@swarup4741
Copy link

swarup4741 commented Jul 16, 2021

Can i work on this issue ?

Hi @swarup4741 - absolutely! Thanks, I'll assign this to you. If for some reason you're unable to pursue it further, please un-assign yourself so others can pick it up.

A good place to start is to look through https://github.com/brave/brave-core. Cheers!

@stephendonner I think @deepxcode addressed this issue on #16939 and his changes looks fine to me ...

@namangirdhar16
Copy link

Hi @stephendonner can i work on this issue, if hasn't fixed yet?

@bsclifton
Copy link
Member

Hi @namangirdhar16 - thanks for the offer; Looks like @deepxcode has the fix in #16939

Thanks @swarup4741 for the assist (ex: reviewing, suggestions in PR). We just need a rebase and then it's good to go

@simonhong
Copy link
Member

Re-opened as it's reverted (brave/brave-core#9970).

@simonhong simonhong reopened this Sep 3, 2021
@bsclifton bsclifton removed this from the 1.31.x - Nightly milestone Sep 3, 2021
@rebron
Copy link
Collaborator

rebron commented May 18, 2022

cc: @Tonev Something you can take a look at to fix?

@Tonev
Copy link
Contributor

Tonev commented May 19, 2022

Hey, @rebron !

I don't know how to proceed in such cases where you have the same string, with the same ID, for uses for upper and lowercase. I'm pretty sure there is a logic for that, because I can't reproduce the issue @stephendonner has reported, so there is definitely something that decides whether strings are displayed in upper or lowercase.

image

I will appreciate it if someone can explain why we need the upper/lowercase checks and how to proceed in cases like this one, so I can help you fix such string differences in the future. I can't simply delete the message because you'll have to revert the changes once again for the message ID will not be found.

@stephendonner
Copy link
Author

Hey, @rebron !

I don't know how to proceed in such cases where you have the same string, with the same ID, for uses for upper and lowercase. I'm pretty sure there is a logic for that, because I can't reproduce the issue @stephendonner has reported, so there is definitely something that decides whether strings are displayed in upper or lowercase.

image

I will appreciate it if someone can explain why we need the upper/lowercase checks and how to proceed in cases like this one, so I can help you fix such string differences in the future. I can't simply delete the message because you'll have to revert the changes once again for the message ID will not be found.

@Tonev mac uses "Title Case" and Windows and Linux use "Sentence Case" - does that help?

@Tonev
Copy link
Contributor

Tonev commented May 28, 2022

Thank you, @stephendonner!

I'm still a bit lost here though. Does it mean editing the string that is inside the titlecase check will be enough to resolve this issue? I can't fully understand why there is a separate, different message if we want to achieve the same result on all OS's.

@stephendonner
Copy link
Author

Thank you, @stephendonner!

I'm still a bit lost here though. Does it mean editing the string that is inside the titlecase check will be enough to resolve this issue? I can't fully understand why there is a separate, different message if we want to achieve the same result on all OS's.

Sorry for the delay; yes, @Tonev editing that should be enough, I think - macOS human-interface-guidelines say to use "Title Case" (capitalizes just like book or movie titles, etc.), and Windows and Linux use "Sentence case" for menus in the UI.

@fallaciousreasoning
Copy link

@sangwoo108 has fixed this!
#23179

@stephendonner
Copy link
Author

Verified PASSED using

Brave 1.43.58 Chromium: 104.0.5112.81 (Official Build) beta (x86_64)
Revision 5b7b76419d50f583022568b6764b630f6ddc9208-refs/branch-heads/5112@{#1309}
OS macOS Version 11.6.8 (Build 20G730)

Screen Shot 2022-08-05 at 4 40 36 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-text-change This change requires some careful wording. OS/Desktop OS/macOS polish Nice to have — usually related to front-end/visual tasks priority/P4 Planned work. We expect to get to it "soon". QA Pass-macOS QA/Yes release-notes/exclude
Projects
None yet
10 participants