organicmaps/organicmaps-tmp

Archived

Improve desktop place page dialog #7505

Merged

Ferenc- merged 12 commits from improve-desktop-place-page-dialog into master

2024-03-23 21:56:49 +00:00

Ferenc- commented

2024-03-02 16:03:15 +00:00

(Migrated from github.com)

No description provided.

biodranik (Migrated from github.com) reviewed 2024-03-22 23:41:39 +00:00

biodranik (Migrated from github.com) left a comment

Awesome, thank you! Maybe Wikipedia window size can be increased for some small size Place Pages.

Awesome, thank you! Maybe Wikipedia window size can be increased for some small size Place Pages.

qt/main.cpp

biodranik (Migrated from github.com) commented

2024-03-05 14:10:18 +00:00

    settings::Set(settings::kDeveloperMode, developerMode);

```suggestion settings::Set(settings::kDeveloperMode, developerMode); ```

biodranik (Migrated from github.com) commented

2024-03-05 14:17:28 +00:00

#ifdef DEBUG
  static bool constexpr kDeveloperMode = true;
#else
  static bool constexpr kDeveloperMode = false;
#endif

```suggestion #ifdef DEBUG static bool constexpr kDeveloperMode = true; #else static bool constexpr kDeveloperMode = false; #endif ```

qt/place_page_dialog.cpp

biodranik (Migrated from github.com) commented

2024-03-05 14:18:21 +00:00

Why include names introduced in a different code style than above?

Why include names introduced in a different code style than above?

biodranik (Migrated from github.com) commented

2024-03-05 14:20:10 +00:00

No global constants please. They can clash with Unity builds and pollute the namespace.
static int constexpr kMaxLengthOfPlacePageDescription = 500;

1. No global constants please. They can clash with Unity builds and pollute the namespace. 2. `static int constexpr kMaxLengthOfPlacePageDescription = 500;`

qt/place_page_dialog_user.cpp

biodranik (Migrated from github.com) commented

2024-03-22 23:17:01 +00:00

No global vars please. Put them into an anonymous namespace.

int constexpr kMaxLengthOfPlacePageDescription = 500;
int constexpr kMinWidthOfShortDescription = 390;

No global vars please. Put them into an anonymous namespace. ```suggestion int constexpr kMaxLengthOfPlacePageDescription = 500; int constexpr kMinWidthOfShortDescription = 390; ```

biodranik (Migrated from github.com) commented

2024-03-22 23:25:38 +00:00

Note formatting and the absence of memory allocations.

namespace
{
std::string_view stripSchemeFromURI(std::string_view uri) {
  for (std::string_view prefix : {"https://", "http://"})
  {
    if (strings::StartsWith(uri, prefix))
      return uri.substr(prefix.size());
  }
  return uri;
}
}  // namespace

Note formatting and the absence of memory allocations. ```suggestion namespace { std::string_view stripSchemeFromURI(std::string_view uri) { for (std::string_view prefix : {"https://", "http://"}) { if (strings::StartsWith(uri, prefix)) return uri.substr(prefix.size()); } return uri; } } // namespace ```

biodranik (Migrated from github.com) commented

2024-03-22 23:27:08 +00:00

Where is it used? Can it be rewritten with string_view?

Where is it used? Can it be rewritten with string_view?

biodranik (Migrated from github.com) commented

2024-03-22 23:27:23 +00:00

indent?

indent?

biodranik (Migrated from github.com) commented

2024-03-22 23:29:18 +00:00

As we're using BSD style for bracers, there's no need to introduce K&R one everywhere.

    if (auto facebook = info.GetMetadata(feature::Metadata::EType::FMD_CONTACT_FACEBOOK); !facebook.empty())
    {

As we're using BSD style for bracers, there's no need to introduce K&R one everywhere. ```suggestion if (auto facebook = info.GetMetadata(feature::Metadata::EType::FMD_CONTACT_FACEBOOK); !facebook.empty()) { ```

biodranik (Migrated from github.com) commented

2024-03-22 23:31:37 +00:00

Is it possible to avoid duplication for social networks by creating a lambda that takes a pair of parameters: "VK" and FMD_CONTACT_VK ? Can addEntry lambda be reused?

Is it possible to avoid duplication for social networks by creating a lambda that takes a pair of parameters: "VK" and FMD_CONTACT_VK ? Can `addEntry` lambda be reused?

biodranik (Migrated from github.com) commented

2024-03-22 23:35:36 +00:00

One liners without bracers here and in other places.

    if (auto level = info.GetMetadata(feature::Metadata::EType::FMD_LEVEL); !level.empty())
      addEntry("Level", std::string(level));

One liners without bracers here and in other places. ```suggestion if (auto level = info.GetMetadata(feature::Metadata::EType::FMD_LEVEL); !level.empty()) addEntry("Level", std::string(level)); ```

biodranik (Migrated from github.com) commented

2024-03-22 23:36:29 +00:00

Btw, did you try to use git-clang-format? Make a commit before doing it.

Btw, did you try to use git-clang-format? Make a commit before doing it.

qt/preferences_dialog.cpp

biodranik (Migrated from github.com) commented

2024-03-22 23:37:16 +00:00

```suggestion ```

Ferenc- (Migrated from github.com) reviewed 2024-03-23 09:02:04 +00:00

qt/main.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 09:02:04 +00:00

done

done

Ferenc- (Migrated from github.com) reviewed 2024-03-23 09:02:19 +00:00

qt/main.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 09:02:19 +00:00

done

done

Ferenc- (Migrated from github.com) reviewed 2024-03-23 09:03:08 +00:00

qt/place_page_dialog_user.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 09:03:08 +00:00

done

done

Ferenc- (Migrated from github.com) reviewed 2024-03-23 09:05:26 +00:00

qt/place_page_dialog_user.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 09:05:26 +00:00

done

done

Ferenc- (Migrated from github.com) reviewed 2024-03-23 09:06:52 +00:00

qt/place_page_dialog_user.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 09:06:52 +00:00

done

done

Ferenc- (Migrated from github.com) reviewed 2024-03-23 09:09:24 +00:00

qt/preferences_dialog.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 09:09:24 +00:00

removed

removed

Ferenc- (Migrated from github.com) reviewed 2024-03-23 12:03:30 +00:00

qt/place_page_dialog_user.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 12:03:30 +00:00

done

done

Ferenc- (Migrated from github.com) reviewed 2024-03-23 12:04:55 +00:00

qt/place_page_dialog_user.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 12:04:55 +00:00

Removed.

Removed.

Ferenc- (Migrated from github.com) reviewed 2024-03-23 12:07:23 +00:00

qt/place_page_dialog.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 12:07:23 +00:00

done

done

Ferenc- (Migrated from github.com) reviewed 2024-03-23 12:12:00 +00:00

qt/place_page_dialog_user.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 12:12:00 +00:00

Braces are done. I will check the tool

Braces are done. I will check the tool

Ferenc- (Migrated from github.com) reviewed 2024-03-23 12:35:06 +00:00

qt/place_page_dialog_user.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 12:35:05 +00:00

I have created a new lambda for this.

I have created a new lambda for this.

Ferenc- (Migrated from github.com) reviewed 2024-03-23 14:38:18 +00:00

qt/place_page_dialog.cpp

Ferenc- (Migrated from github.com) commented

2024-03-23 14:38:18 +00:00

Fixed

Fixed

biodranik (Migrated from github.com) approved these changes 2024-03-23 21:54:20 +00:00

biodranik (Migrated from github.com) left a comment

Thank you, @Ferenc- , for your patience and for getting things done. I appreciate it a lot. The code is way cleaner now and easier to read.

Thanks to @dmfrodrigues too for starting this important work.

Thank you, @Ferenc- , for your patience and for getting things done. I appreciate it a lot. The code is way cleaner now and easier to read. Thanks to @dmfrodrigues too for starting this important work.

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

No reviewers

No labels

Android Automotive (AAOS)

Android Automotive (AAOS)

Battery and Performance

Battery and Performance

Bookmarks and Tracks

Bookmarks and Tracks

External Map Datasets

External Map Datasets

Frequently User Reported

Frequently User Reported

Good first issue

Good first issue

Linux packaging

Linux packaging

NLnet 2024-06-281

NLnet 2024-06-281

No Feature Parity

No Feature Parity

Public Transport

Public Transport

Track Recording

Track Recording

Walk Navigation

Walk Navigation

No milestone

No project

No assignees

1 participant

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#7505

No description provided.