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.

Sign up for GitHub

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

Jump to bottom

[Android] Not all settings have close button #10375

Closed
samartnik opened this issue Jun 19, 2020 · 3 comments · Fixed by brave/brave-core#8702
Closed

[Android] Not all settings have close button #10375

samartnik opened this issue Jun 19, 2020 · 3 comments · Fixed by brave/brave-core#8702
Assignees
@samartnik
Labels
OS/Android Fixes related to Android browser functionality QA Pass - Android ARM QA Pass - Android Tab QA/Test-Plan-Specified QA/Yes release-notes/include
Milestone
1.25.x - Release

Comments

@samartnik
Copy link
Contributor

samartnik commented Jun 19, 2020 •
edited by srirambv
Loading

Description

Some settings don't have close button

device-2020-06-18-181120 device-2020-06-18-181139

In cr84 there were changed 2 more options

device-2020-06-18-181338 device-2020-06-18-181358

For consistency we should replace menu_help in chrome/android/java/src/org/chromium/chrome/browser/settings/SettingsActivity.java with close button. This way any new or changed settings will have it by default.

Steps to reproduce

  1. See pictures in description

Actual result

Not all settings have close button

Expected result

Close button in all settings

Issue reproduces how often

Always

Issue happens on

  • Current Play Store version? yes
  • Beta build? yes

Device details

  • Install type (ARM, x86):
  • Device (Phone, Tablet, Phablet):
  • Android version:

Brave version

1.9.80

Website problems only

  • Does the issue resolve itself when disabling Brave Shields?
  • Does the issue resolve itself when disabling Brave Rewards?
  • Is the issue reproducible on the latest version of Chrome?

Additional information
@samartnik samartnik added the OS/Android Fixes related to Android browser functionality label Jun 19, 2020
samartnik added a commit to brave/brave-core that referenced this issue Mar 8, 2021
@samartnik
[Android] LanguageSettings were modularized
4481c09 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mariospr pushed a commit to brave/brave-core that referenced this issue Mar 11, 2021
@samartnik @mariospr
[Android] LanguageSettings were modularized
ef39391 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mariospr pushed a commit to brave/brave-core that referenced this issue Mar 18, 2021
@samartnik @mariospr
[Android] LanguageSettings were modularized
74e0671 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mariospr pushed a commit to brave/brave-core that referenced this issue Mar 22, 2021
@samartnik @mariospr
[Android] LanguageSettings were modularized
98fc890 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mariospr pushed a commit to brave/brave-core that referenced this issue Mar 24, 2021
@samartnik @mariospr
[Android] LanguageSettings were modularized
2586318 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mkarolin pushed a commit to brave/brave-core that referenced this issue Mar 30, 2021
@samartnik @mkarolin
[Android] LanguageSettings were modularized
a51979d 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mkarolin pushed a commit to brave/brave-core that referenced this issue Mar 30, 2021
@samartnik @mkarolin
[Android] Night mode was modularized
2d992f0 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/b8f8625d98d573809cf06674f733ae31e6276eec

Android: Modularize c.b.night_mode

Moves c.b.night_mode files to a new moduarlized target chrome/browser/
ui/android/night_mode. This helps modularize other packages that
depend on static utils such as NightModeUtils.

Bug: 1171512
mkarolin pushed a commit to brave/brave-core that referenced this issue Apr 7, 2021
@samartnik @mkarolin
[Android] LanguageSettings were modularized
9984422 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mkarolin pushed a commit to brave/brave-core that referenced this issue Apr 7, 2021
@samartnik @mkarolin
[Android] Night mode was modularized
a20a61b 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/b8f8625d98d573809cf06674f733ae31e6276eec

Android: Modularize c.b.night_mode

Moves c.b.night_mode files to a new moduarlized target chrome/browser/
ui/android/night_mode. This helps modularize other packages that
depend on static utils such as NightModeUtils.

Bug: 1171512
mariospr pushed a commit to brave/brave-core that referenced this issue Apr 9, 2021
@samartnik @mariospr
[Android] LanguageSettings were modularized
428ee62 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mariospr pushed a commit to brave/brave-core that referenced this issue Apr 9, 2021
@samartnik @mariospr
[Android] Night mode was modularized
9925920 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/b8f8625d98d573809cf06674f733ae31e6276eec

Android: Modularize c.b.night_mode

Moves c.b.night_mode files to a new moduarlized target chrome/browser/
ui/android/night_mode. This helps modularize other packages that
depend on static utils such as NightModeUtils.

Bug: 1171512
mariospr pushed a commit to brave/brave-core that referenced this issue Apr 9, 2021
@samartnik @mariospr
[Android] LanguageSettings were modularized
016ae44 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mariospr pushed a commit to brave/brave-core that referenced this issue Apr 9, 2021
@samartnik @mariospr
[Android] Night mode was modularized
44e9b43 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/b8f8625d98d573809cf06674f733ae31e6276eec

Android: Modularize c.b.night_mode

Moves c.b.night_mode files to a new moduarlized target chrome/browser/
ui/android/night_mode. This helps modularize other packages that
depend on static utils such as NightModeUtils.

Bug: 1171512
mkarolin pushed a commit to brave/brave-core that referenced this issue Apr 27, 2021
@samartnik @mkarolin
[Android] LanguageSettings were modularized
502aa17 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mkarolin pushed a commit to brave/brave-core that referenced this issue Apr 27, 2021
@samartnik @mkarolin
[Android] Night mode was modularized
80b4824 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/b8f8625d98d573809cf06674f733ae31e6276eec

Android: Modularize c.b.night_mode

Moves c.b.night_mode files to a new moduarlized target chrome/browser/
ui/android/night_mode. This helps modularize other packages that
depend on static utils such as NightModeUtils.

Bug: 1171512
mkarolin pushed a commit to brave/brave-core that referenced this issue Apr 30, 2021
@samartnik @mkarolin
[Android] LanguageSettings were modularized
df7dc05 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mkarolin pushed a commit to brave/brave-core that referenced this issue Apr 30, 2021
@samartnik @mkarolin
[Android] Night mode was modularized
bc762aa 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/b8f8625d98d573809cf06674f733ae31e6276eec

Android: Modularize c.b.night_mode

Moves c.b.night_mode files to a new moduarlized target chrome/browser/
ui/android/night_mode. This helps modularize other packages that
depend on static utils such as NightModeUtils.

Bug: 1171512
@samartnik samartnik mentioned this issue May 4, 2021
Merged
24 tasks
@samartnik samartnik mentioned this issue May 4, 2021
Open
@samartnik
Copy link
Contributor Author

Some options declare their own menu, so we will need to separately add close button there. Follow up issue created #15615

@samartnik
Copy link
Contributor Author

Test plan: check that all options, apart from options in follow up issue above, have close button.

@samartnik samartnik self-assigned this May 4, 2021
@samartnik samartnik added this to the 1.26.x - Nightly milestone May 4, 2021
mkarolin pushed a commit to brave/brave-core that referenced this issue May 5, 2021
@samartnik @mkarolin
[Android] LanguageSettings were modularized
e0b41e0 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mkarolin pushed a commit to brave/brave-core that referenced this issue May 5, 2021
@samartnik @mkarolin
[Android] Night mode was modularized
c40d9d3 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/b8f8625d98d573809cf06674f733ae31e6276eec

Android: Modularize c.b.night_mode

Moves c.b.night_mode files to a new moduarlized target chrome/browser/
ui/android/night_mode. This helps modularize other packages that
depend on static utils such as NightModeUtils.

Bug: 1171512
mariospr pushed a commit to brave/brave-core that referenced this issue May 5, 2021
@samartnik @mariospr
[Android] LanguageSettings were modularized
ebb792e 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/f3c257e9298faf4da38daa674571c7ad08873106
[Android] Move LanguageSettings to //chrome/browser/language

This CL moves LangaugeSettings.java and the associated XML files out of
//chrome/android to the //chrome/browser/language module.

There are no UI changes from this CL.  A follow up CL will move the
language related tests to //chrome/browser/language.

Bug: 1124096
mariospr pushed a commit to brave/brave-core that referenced this issue May 5, 2021
@samartnik @mariospr
[Android] Night mode was modularized
11e2aeb 
The only reason we used BravePreferenceFragment is to have a close button in the
settings. This will be re-done in the context of this issue brave/brave-browser#10375

Chromium change:
https://chromium.googlesource.com/chromium/src.git/+/b8f8625d98d573809cf06674f733ae31e6276eec

Android: Modularize c.b.night_mode

Moves c.b.night_mode files to a new moduarlized target chrome/browser/
ui/android/night_mode. This helps modularize other packages that
depend on static utils such as NightModeUtils.

Bug: 1171512
@mkarolin mkarolin mentioned this issue May 18, 2021
Merged
24 tasks
@srirambv
Copy link
Contributor

Verification passed on OnePlus 6T with Android 10 running 1.25.65 x64 build

  • Verified close button is available on all menu settings
  • Verified clicking it closes the menu and goes back to tab
image image image image

Verification passed on Samsung Tab A with Android 10 running 1.25.65 x64 build

  • Verified close button is available on all menu settings
  • Verified clicking it closes the menu and goes back to tab
image image image image

This was referenced May 27, 2021
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@samartnik samartnik

Labels
OS/Android Fixes related to Android browser functionality QA Pass - Android ARM QA Pass - Android Tab QA/Test-Plan-Specified QA/Yes release-notes/include
Projects
None yet
Milestone
1.25.x - Release
Development

Successfully merging a pull request may close this issue.

[Android] Changed the way we handle close button in settings brave/brave-core
3 participants
@kjozwiak @srirambv @samartnik