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

Opening hours: interface "jumps" when selecting closing time hour #4763

Closed
pinkdroyd opened this issue Jan 24, 2023 · 13 comments
Closed

Opening hours: interface "jumps" when selecting closing time hour #4763

pinkdroyd opened this issue Jan 24, 2023 · 13 comments
Labels

Comments

@pinkdroyd
Copy link

Missing opening hours quest: At the moment when I click on the hour for closing time the checkbox for open end appears below the hour radial selector. With that the radial hour selector slightly moves up which leads to a misclick and a wrong hour entered.

How to Reproduce
Select the quest to add missing opening hours. Enter opening time and click on next. Click on an hour for closing time.

Expected Behavior
Checkbox for open end is visible from the start and does not appear during clicking. That way the UI does not shift and the correct hour is entered.

Versions affected
Android 10 (security updates from Aug 2022)
StreetComplete v50.2 installed from GooglePlay
This problem has existed for longer than the current version.

@pinkdroyd pinkdroyd added the bug label Jan 24, 2023
@mnalis
Copy link
Member

mnalis commented Jan 27, 2023

Indeed it does, now that I look closely, I can confirm on Android 10 / SC v50.2 !

So that's why I almost always hit opening times OK, but often miss when clicking closing times! I always though that were just my fat fingers, but here is the evidence that that is indeed this bug:

small_SVID_20230127_011519_1.mp4

If you look frame by frame, you'll see that I clearly clicked on closing time 21 hour and it shows in the form, but then before I managed to remove my finger from the screen the form scrolled, and changed to 20 instead!

Great catch @pinkdroyd , this has been driving me nuts but I never noticed that it scrolls!

@matkoniecz
Copy link
Member

It seems related to how "open end" checkbox appears. I guess that it can be added earlier?

@pinkdroyd
Copy link
Author

From a usability perspective it makes the most sense to always show the "open end" checkbox in the closing time screen. Right now you have to enter a (wrong) time just to make the checkbox appear and then be able to click on the checkbox.
So I think fixing this bug solves two issues at once.

@matkoniecz
Copy link
Member

oh, that would be extra thing to improve

the selected hour combined with open end means "it will be definitely open by that time, likely open also later"

@arrival-spring
Copy link
Contributor

In version 51 I am still experiencing the jump as shown in the video at the top of this issue.

@westnordost westnordost reopened this Feb 12, 2023
@westnordost
Copy link
Member

Uhh, so the issue seems to be that the "open end" checkbox is hidden for some reason before the time view is tapped

@Helium314
Copy link
Collaborator

You could always have the checkbox in the view ("start" and "end"), but set it invisible at start, then set it to visible when switching to "end".
There is some initially useless empty space, but the jump is gone then.

@westnordost
Copy link
Member

How's that possible? The start and end views are two separate layouts.

@Helium314
Copy link
Collaborator

You can simply use the same layout for both then.

@Helium314
Copy link
Collaborator

If such a large empty space is acceptable, I can do a PR
Screenshot_20230216-213825_SCEE

@mnalis
Copy link
Member

mnalis commented Feb 16, 2023

If such a large empty space is acceptable

It has that large empty space for me currently in 51.0 when I switch back from closing time to opening time (but yeah: on first form open, it does not have that large empty space)

@westnordost
Copy link
Member

I would like to have the issue solved at the root cause for this. If possible. I.e. why does the end-layout initially not show the view? After all, it is not set to gone at the moment.

Also, if you do a PR, it should probably revert the earlier attempt to fix this which made it so that the picker would be cut off in landscape mode, if I remember correctly (by removing the scrollview. I thought that would be the cause of this issue).

@westnordost
Copy link
Member

Anyway, I tried some things, but to no avail. I think the main issue is that the ViewPager2 works with a RecyclerView and a RecyclerView does not expect its elements to have different dimensions (in this case: height). So, I gave up and basically implemented what @Helium314 suggested.

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

No branches or pull requests

6 participants