organicmaps/organicmaps-tmp

Archived

Fork 1

Improve page dialog (user) #8224

Merged

dmfrodrigues merged 9 commits from improve-page-dialog-user into master

2024-05-23 08:23:02 +00:00

dmfrodrigues commented

2024-05-19 19:05:08 +00:00

(Migrated from github.com)

Adds short description to place page dialog.
Adds word-wrap for all long fields.
Bug fix: removes duplicate Instagram.
Adds some layout stretching.

Points 2 and 4 are in preparation to move the page dialog to a panel to the left of the map 🙂 (which I'll do in a separate PR). This will hopefully make the place page less of a nuisance when one specific place is selected but you still want to explore the surroundings, giving a more intuitive experience that is also consistent with the mobile version (where you can select a place, and then drag the map around to see the surroundings).

Before	After

@biodranik @Ferenc-

1. Adds short description to place page dialog. 2. Adds word-wrap for all long fields. 3. Bug fix: removes duplicate Instagram. 4. Adds some layout stretching. Points 2 and 4 are in preparation to move the page dialog to a panel to the left of the map :slightly_smiling_face: (which I'll do in a separate PR). This will hopefully make the place page less of a nuisance when one specific place is selected but you still want to explore the surroundings, giving a more intuitive experience that is also consistent with the mobile version (where you can select a place, and then drag the map around to see the surroundings). | Before | After | |-|-| | ![image](https://github.com/organicmaps/organicmaps/assets/43684166/24f696d4-8ed5-420b-9afa-ec1ba4f597ce) | ![image](https://github.com/organicmaps/organicmaps/assets/43684166/53bc96bc-663f-48f8-bccc-063ed69dfa6a) | @biodranik @Ferenc-

👍 1

Ferenc- (Migrated from github.com) approved these changes 2024-05-22 07:42:28 +00:00

Ferenc- (Migrated from github.com) left a comment

LGTM

biodranik (Migrated from github.com) reviewed 2024-05-22 20:32:56 +00:00

biodranik (Migrated from github.com) left a comment

A few minor nits.

qt/place_page_dialog_user.cpp

biodranik (Migrated from github.com) commented

2024-05-22 20:31:02 +00:00

  auto const paragraphStart = description.find("<p>");
  auto const paragraphEnd = description.find("</p>");

```suggestion auto const paragraphStart = description.find("<p>"); auto const paragraphEnd = description.find("</p>"); ```

biodranik (Migrated from github.com) commented

2024-05-22 20:31:14 +00:00

    description = description.substr(3, paragraphEnd - 3);

```suggestion description = description.substr(3, paragraphEnd - 3); ```

biodranik (Migrated from github.com) commented

2024-05-22 20:31:59 +00:00

    description = description.substr(0, kMaxLengthOfPlacePageDescription - 3) + "...";

```suggestion description = description.substr(0, kMaxLengthOfPlacePageDescription - 3) + "..."; ```

biodranik (Migrated from github.com) commented

2024-05-22 20:32:48 +00:00

There are many memory allocations here for string processing, but... ok.

qt/place_page_dialog_user.cpp

biodranik (Migrated from github.com) commented

2024-05-22 20:27:38 +00:00

    if (auto const addressFormatted = address.FormatAddress(); !addressFormatted.empty())
    {

```suggestion if (auto const addressFormatted = address.FormatAddress(); !addressFormatted.empty()) { ```

biodranik (Migrated from github.com) commented

2024-05-22 20:27:56 +00:00

    if (auto const subTitle = info.GetSubtitle(); !subTitle.empty())
    {

```suggestion if (auto const subTitle = info.GetSubtitle(); !subTitle.empty()) { ```

biodranik (Migrated from github.com) commented

2024-05-22 20:28:07 +00:00

    if (!title.empty())
    {

```suggestion if (!title.empty()) { ```

dmfrodrigues (Migrated from github.com) reviewed 2024-05-22 20:46:56 +00:00

qt/place_page_dialog_user.cpp

dmfrodrigues (Migrated from github.com) commented

2024-05-22 20:46:56 +00:00

Addressed using string_view. IMO not as pretty, but it's two less allocations.

biodranik (Migrated from github.com) reviewed 2024-05-22 20:58:53 +00:00

qt/place_page_dialog_user.cpp

biodranik (Migrated from github.com) commented

2024-05-22 20:58:52 +00:00

If string_view is used, can it be passed as an argument, and returned from the function, to avoid two more allocations?

dmfrodrigues (Migrated from github.com) reviewed 2024-05-22 21:46:15 +00:00

qt/place_page_dialog_user.cpp

dmfrodrigues (Migrated from github.com) commented

2024-05-22 21:46:15 +00:00

Most likely can't return a view, since we need to add "..." at the end. But passing description as a const ref to this function would remove one more allocation.