WIP: add collection_times (text-only display) #7950

Draft

RedAuburn wants to merge 2 commits from RedAuburn/collection_times-support into master

RedAuburn commented

2024-04-18 15:58:57 +00:00

Member

Initial part of organicmaps/organicmaps#2431, pending the opening_hours parser re-write (organicmaps/organicmaps#7974)

<img width="300px" src="https://github.com/organicmaps/organicmaps/assets/26939824/cebd2bbd-2d5f-4556-a2f8-77635a04c3fd"/> Initial part of https://git.omaps.dev/organicmaps/organicmaps/issues/2431, pending the opening_hours parser re-write (https://git.omaps.dev/organicmaps/organicmaps/issues/7974)

👍 2

vng (Migrated from github.com) reviewed 2024-04-18 15:58:57 +00:00

biodranik (Migrated from github.com) reviewed 2024-04-24 22:25:45 +00:00

biodranik (Migrated from github.com) left a comment

Will it be displayed properly in the UI? Does our UI support displaying single-time values, not ranges?

And what about the editor? It should have a different UI, right?

Which OSM objects have this feature?

Will it be displayed properly in the UI? Does our UI support displaying single-time values, not ranges? And what about the editor? It should have a different UI, right? Which OSM objects have this feature?

biodranik (Migrated from github.com) approved these changes 2024-04-26 19:35:58 +00:00

biodranik (Migrated from github.com) left a comment

I'm ok to add it to generator now and properly add into the UI/editor later. @vng WDYT?

biodranik (Migrated from github.com) reviewed 2024-07-01 21:59:59 +00:00

biodranik (Migrated from github.com) left a comment

Thanks! Can the same rendering for PP cell as for the opening hours be used instead of a plain string? Or does the OH rendering only work with ranges?

And is it hard to enable the Editor?

@euf maybe a hand holding a clock can distinguish opening hours vs collection times?

@kirylkaveryn would it be feasible to render "Collection times" small text on the cell like on the screenshot?

Thanks! Can the same rendering for PP cell as for the opening hours be used instead of a plain string? Or does the OH rendering only work with ranges? And is it hard to enable the Editor? @euf maybe a hand holding a clock can distinguish opening hours vs collection times? @kirylkaveryn would it be feasible to render "Collection times" small text on the cell like on the screenshot?

data/strings/strings.txt

biodranik (Migrated from github.com) commented

2024-07-01 20:42:10 +00:00

    uk = Час збору

@strump right?

```suggestion uk = Час збору ``` @strump right?

feature_list/feature_list.cpp

biodranik (Migrated from github.com) commented

2024-07-01 20:45:01 +00:00

Would it be better to print collection_times closer to opening_hours?

vng (Migrated from github.com) reviewed 2024-07-02 11:48:33 +00:00

feature_list/feature_list.cpp

vng (Migrated from github.com) commented

2024-07-02 11:48:33 +00:00

No, please, keep as-is in the end. Or skip this change at all.

biodranik (Migrated from github.com) approved these changes 2024-07-02 17:33:02 +00:00

biodranik (Migrated from github.com) left a comment

i don't think the OH table works with points instead of ranges yet, but it's definitely something that should be added. Hopefully when i have more time.

Have you already tried it? Let's create an issue for that.

Another issue to create and fix later is to show the opening times collapsed by default, like on iOS (so only the Today line is visible), and only expand the opening hours/collection times if a user taps on the cell.

LGTM, @vng PTAL

> i don't think the OH table works with points instead of ranges yet, but it's definitely something that should be added. Hopefully when i have more time. Have you already tried it? Let's create an issue for that. Another issue to create and fix later is to show the opening times collapsed by default, like on iOS (so only the Today line is visible), and only expand the opening hours/collection times if a user taps on the cell. LGTM, @vng PTAL

This repo is archived. You cannot comment on pull requests.

No reviewers

vng

biodranik