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

SCEE setting submenu lacking back button and header text #640

Closed
HolgerJeromin opened this issue Sep 5, 2024 · 10 comments
Closed

SCEE setting submenu lacking back button and header text #640

HolgerJeromin opened this issue Sep 5, 2024 · 10 comments
Labels
bug Something isn't working

Comments

@HolgerJeromin
Copy link

HolgerJeromin commented Sep 5, 2024

SCEE 59.0-alpha2 on Android 14 on a Motorola device.

The SCEE specific setting submenus have no back icon on the top left:

Screenshot_20240905-075051

Note: The first "row" is correctly blank in the screenshot as it is covered by android menu.

@HolgerJeromin HolgerJeromin added the bug Something isn't working label Sep 5, 2024
@mnalis
Copy link
Collaborator

mnalis commented Sep 5, 2024

I can confirm missing back button even on SCEE 58.2 on my Android 14 (Samsung Galaxy S23+), both with navigation buttons and with gesture mode:

small_Screenshot_20240905_143555_SCEE small_Screenshot_20240906_022520_SCEE

@rusty-snake
Copy link

rusty-snake commented Sep 5, 2024

WFM

  • SCEE 58.22
  • AOSP 14 QPR3
  • Gesture Navigation

Related: #613

@mnalis

This comment was marked as outdated.

@mnalis
Copy link
Collaborator

mnalis commented Sep 6, 2024

On Android 14, Xiaomi Redmi Note 13, it is broken in 58.2 and 59.0-alpha2:
small_Screenshot_2024-09-06-02-41-37-204_de westnordost streetcomplete expert small_Screenshot_2024-09-06-02-35-54-044_de westnordost streetcomplete expert

but works in 58.21 and 58.22 :
small_Screenshot_2024-09-06-02-46-59-841_de westnordost streetcomplete expert small_Screenshot_2024-09-06-02-36-54-708_de westnordost streetcomplete expert

So I guess it was fixed somewhere in between, but have not made it into v59.0-alpha2 (and there have been no further SCEE releases after alpha2 which include it, and latest SCEE master fails to build so can't test it there easily).

@mcliquid
Copy link

mcliquid commented Sep 6, 2024

Relevant PRs:

I guess it has been deactivated here: ec0660e#diff-de7525ef2b31d51f2f9a717ec1c5ce00795b546dd348b81318d6d5b07e794ab2

@mnalis
Copy link
Collaborator

mnalis commented Sep 6, 2024

I guess it has been deactivated here: ec0660e#diff-de7525ef2b31d51f2f9a717ec1c5ce00795b546dd348b81318d6d5b07e794ab2

Thanks for that research @mcliquid, it seems it is still unfixed in current master 38fce28 then, so this issue should remain open:

% fgrep -r setUpToolbarTitleAndIcon
fgrep -r setUpToolbarTitleAndIcon
DataManagementSettingsFragment.kt://            setUpToolbarTitleAndIcon(this)
DisplaySettingsFragment.kt://            setUpToolbarTitleAndIcon(this)
QuestsSettingsFragment.kt://            setUpToolbarTitleAndIcon(this)
UiSettingsFragment.kt://            setUpToolbarTitleAndIcon(this)
NoteSettingsFragment.kt://            setUpToolbarTitleAndIcon(this)

@mcliquid
Copy link

mcliquid commented Sep 6, 2024

@mnalis I have re-added the relevant changes locally and cannot find any issues. The implementation works perfectly for me.
@Helium314 any reasons against these changes?

image

@Helium314
Copy link
Owner

The main issue is that this feature keeps creating merge conflicts, and I don't want to spend even more time on merging upstream just for an (in my opinion) irrelevant extra.

@Helium314
Copy link
Owner

Also it would only be temporary, until SCEE settings screens are migrated to compose.

@mnalis
Copy link
Collaborator

mnalis commented Oct 10, 2024

It was fixed by PR #668; but see comments in #668 (comment)

@mnalis mnalis closed this as completed Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants