[android] Add website:menu tag support

[Arthur-GYT]

All is in the title ^^.

Close #2376

I've tested this PR on this node (France_Free County_North.mwm) : https://www.openstreetmap.org/node/6637693465

I leave it to those who wish (and can) to add this to the IOS version. As I don't own a Mac, it's impossible for me to take care of this ^^.

[Arthur-GYT]

I can't test it, I don't find (and know) any restaurant with this tag in the current state of the map (march 2024). Or maybe I do a mistake. Any suggestion ?

[Jean-BaptisteC]

Good feature
That's require a map generation, to see this tag in place page

[Arthur-GYT]

Good feature
That's require a map generation, to see this tag in place page

Okay, I will take a look. Just one more thing, I don't need to commit map generation, right ?

[RedAuburn]

Okay, I will take a look. Just one more thing, I don't need to commit map generation, right ?

nope 👍️

[Arthur-GYT]

Ready for review ^^ !!

[RedAuburn]

can you post a screenshot of how it looks? :)

[RedAuburn]

looks good overall! a small nit:

[Arthur-GYT]

can you post a screenshot of how it looks? :)

It's uploaded in my first message ^^.

[RedAuburn]

should it show the link to the menu, or would it be better to use the text "Open Menu in Browser"?
the menu link in the screenshot looks good, but often it's a messy link to a PDF:

eg.
https://www.openstreetmap.org/node/10271116595

where the link is (or was at least)
http://79.170.40.239/devthecoriander.com/wp-content/uploads/2020/01/The-Coriander-Buckhurst-Hill_Essex-take-away-menu-proof.pdf

[biodranik]

1. Cuisine icons and menu icons are very similar and confusing. They should be distinct and clear.
2. As it is a menu web URL, then it should clearly say "Menu website link" or "Menu link" or something similar in the editor.
3. Both cuisine and menu should be nearby in POI (they are closely related) and in the editor (we really want to encourage users to add menus, not wifis or something else, this is very valuable data for planning a visit to a restaurant).
4. In the Place Page it is not clear what does an icon and a website URL mean. I suggest replacing it with a clear text: Check Menu.

[Arthur-GYT]

@biodranik I updated the code with your recommendations ^^.

[Arthur-GYT]

should it show the link to the menu, or would it be better to use the text "Open Menu in Browser"? the menu link in the screenshot looks good, but often it's a messy link to a PDF:

eg. https://www.openstreetmap.org/node/10271116595

where the link is (or was at least) http://79.170.40.239/devthecoriander.com/wp-content/uploads/2020/01/The-Coriander-Buckhurst-Hill_Essex-take-away-menu-proof.pdf

I think it's solved with recent changes ^^.

[kirylkaveryn]

@biodranik i need some time to implement this feature in iOS.
Should this PR be merged first or we can wait a little bit?

[biodranik]

Thanks, I'm ok to merge it after a few minor fixes.

@vng PTAL, it would be great to run the generator with these changes.

[biodranik]

Thank you!

@vng merge please

[kirylkaveryn]

@biodranik @Arthur-GYT in iOS we don't have cells with the subtitles in the PP. But without the subtitle, this field may not be clear. Should we add a new cell (it may not be easy because of the current PP implementation) or use the same style as wiki/fb/link?

[biodranik]

@kirylkaveryn Let's make it simpler, by not showing the URL, and improve later if necessary.

[kirylkaveryn]

@Arthur-GYT @biodranik should the new type be validated in the indexer/editable_map_object.cpp? :

bool EditableMapObject::IsValidMetadata(MetadataID type, std::string const & val
{
  switch (type)
  {
  case MetadataID::FMD_WEBSITE: return ValidateWebsite(value);
  case MetadataID::FMD_WEBSITE_MENU: return ValidateWebsite(value); // <--- ?

void EditableMapObject::SetMetadata(MetadataID type, std::string value)
{
  switch (type)
  {
  case MetadataID::FMD_WEBSITE: value = ValidateAndFormat_website(value); break;
  case MetadataID::FMD_WEBSITE_MENU: value = ValidateAndFormat_website(value); break;  // <--- ?
...

[Arthur-GYT]

@kirylkaveryn

I forgot this ^^'. It's added, thanks ^^.

[kirylkaveryn]

@Arthur-GYT also important to add the new key into the editor/editor_config.cpp to te list of fields with metadata. In other case you will get a crash while parsing the editor.config XML tags.

   {"operator", EType::FMD_OPERATOR},
    {"website", EType::FMD_WEBSITE},
    {"website_menu", EType::FMD_WEBSITE_MENU}, // <---
    {"contact_facebook", EType::FMD_CONTACT_FACEBOOK},

And only after that, the editor.config can be updated to support a new tag.
@biodranik @dvdmrtnz can you please correct me if I'm wrong?

[kirylkaveryn]

3. Both cuisine and menu should be nearby in POI (they are closely related) and in the editor (we really want to encourage users to add menus, not wifis or something else, this is very valuable data for planning a visit to a restaurant).

@biodranik
The list of editable objects is autogenerated and the FMD_WEBSITE_MENU is the last in the types enum (46) and manual reordering isn't supported. So in ios it is always appended to the end...

[Arthur-GYT]

@kirylkaveryn It's modified ^^.

[biodranik]

It should be possible to change the order of the visible fields on the screen, the internal order can be anything unrelated.

[kirylkaveryn]

It should be possible to change the order of the visible fields on the screen, the internal order can be anything unrelated.

It requires a refactroing and may be implemented in a separate PR...

[biodranik]

It requires refactoring and may be implemented in a separate PR...

A separate PR is a good idea!

[biodranik]

Ready to merge?

[Arthur-GYT]

For me, yes ^^

[Arthur-GYT]

@biodranik @vng Any news ?

[biodranik]

@Arthur-GYT congrats and thanks for your hard work and contribution!

[patepelo]