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

streamline map wording #2562

Merged
merged 2 commits into from May 15, 2023
Merged

streamline map wording #2562

merged 2 commits into from May 15, 2023

Conversation

r10s
Copy link
Member

@r10s r10s commented May 15, 2023

this pr syncs the wording of "All Location" to what is used elsewhere in the app and common in apps in general (omitting the verb "show" before objects).

see commit messages for more details.

before / after:

   

- remove prefix 'Show', we are not using it at most other places,
  nor do comparable UI

- move 'All Locations' below 'All Media',
  this seems to be more logical
  (we were even thinking of making 'Locations' a tap in 'Media')
@r10s r10s requested review from Hocuri and adbenitez May 15, 2023 08:41
Copy link
Member

@adbenitez adbenitez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better, indeed!

res/values/strings.xml Outdated Show resolved Hide resolved
Copy link
Collaborator

@Hocuri Hocuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the duplicate location string, this LGTM

…s, maybe also the better title for the in-chat-maps in general)
@r10s r10s force-pushed the streamline-map-wording2 branch from 7592057 to 3169a19 Compare May 15, 2023 10:38
@r10s r10s merged commit e50a13f into master May 15, 2023
2 checks passed
@r10s r10s deleted the streamline-map-wording2 branch May 15, 2023 10:40
@@ -157,6 +157,7 @@
<string name="switch_camera">Switch Camera</string>
<string name="toggle_fullscreen">Toggle Full Screen Mode</string>
<string name="location">Location</string>
<string name="locations">Locations</string>
Copy link
Collaborator

@Hocuri Hocuri May 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand, why add this string that seems to be used nowhere?

Is this still by accident and should be removed again in a follow-up PR?

Copy link
Member Author

@r10s r10s May 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is used in some ios experiments, where the word "Map" is not really fitting when showing a number or "None" beside (not yet there, but planned):

   

also, as we're talking about "sending/deleting/all/whatnot locations" also elsewhere, this seems to be a natural fit.

nb: the map is a 50k webxdc with leaflet, if that works out nicely at the end, we can think over using that on android/desktop as well, finally getting rid of mapbox - plus having an option to the user replacing the whole map engine.

however, as said, this is only an experiment currently, with close to no priority these days :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants