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

Better hint that everyone should enable team mode #3172

Merged
merged 3 commits into from
Aug 17, 2021

Conversation

smichel17
Copy link
Member

@smichel17 smichel17 commented Aug 16, 2021

I suspect that people knew they all needed their phones out, but thought they could do things on one phone, because the second step instructions are addressed to "everybody".

This way introduces the "everyone does this" hint sooner, but the second step is directed only at one person; this should make it clear that each person does it individually.

Modified slightly from my suggestion in #3170; this is shorter.

I suspect that people knew they all needed their phones out, but thought
they could do things on one phone, because the second step instructions
are addressed to "everybody".

This way introduces the "everyone does this" hint sooner, but the second
step is directed only at one person; this should make it clear that each
person does it individually.
Comment on lines 871 to 873
<string name="team_mode_team_size_label2">How many people are mapping? Enter the same number on everyone's phone.</string>
<string name="team_mode_team_size_hint">Team mode works for groups of 2 to 12 people.</string>
<string name="team_mode_choose_color2">Now everybody in your group chooses a unique color on their phone:</string>
<string name="team_mode_choose_color2">Next, each person must pick a different color. Choose yours:</string>
Copy link
Member Author

Choose a reason for hiding this comment

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

I went back and forth a lot, about which of the two strings should be "everyone" vs "each person". I think maybe they should be swapped. @peternewman maybe you could give a second opinion?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nothing strong here:
https://www.grammarly.com/blog/everyone-vs-every-one/

There are more hits for "everyone must pick" rather than "each person must pick" (~1.5x), also I'd broadly rate the first page of hits for the former better than the latter.

Although a lot more "on everyone's phone" than "on each person's phone" (> 10x), although I'd broadly rate the latter higher.

I'm guessing we don't want everyone/everybody for both? 😆

In which case I'd probably just favour how it currently is, as the colour is the thing we want to be distinct, and each person feels more directed to the individual.

So now we've got two opinions, we need a third to decide! 🤣

Copy link
Member Author

@smichel17 smichel17 Aug 17, 2021

Choose a reason for hiding this comment

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

I'm guessing we don't want everyone/everybody for both?

Yeah, I was trying not to use the same word twice in a row. I'd be fine with everyone/everybody.

the colour is the thing we want to be distinct, and each person feels more directed to the individual

That's what I was going for.

So now we've got two opinions, we need a third to decide! 🤣

Ok— If we don't care about including "phone", I'd be happy changing the first to, "How many people are mapping? Everyone should enter the same number." Or "They must all enter the same number", if we're going for brevity.

Then I'd be happy to switch to "Next, everyone must pick a different color". Otherwise, I want to stick with "everyone's phone" for brevity.

Copy link
Member Author

Choose a reason for hiding this comment

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

But, at this point I'm happy enough with how it is. If we all are, I'm happy not to bother perfecting the phrasing.

@westnordost
Copy link
Member

westnordost commented Aug 16, 2021

@FloEdelmann @matkoniecz

@matkoniecz
Copy link
Member

I really like the phrasing.

But this time testing was actually useful: there was a compilation error here due to not escaped ' in everyone's

No idea why escaping is needed - and not in say What's also present in the file, but it is not compiling without it.

@smichel17
Copy link
Member Author

Good catch, I forgot to test before pushing. (Maybe a good use case for #1917 :P)

@peternewman
Copy link
Collaborator

Maybe a good use case for #1917 :P

Yeah exactly, it ensures the master branch always compiles and puts the onus on the PR author to fix their submission (as well as catching silly little typos like this).

No idea why escaping is needed - and not in say What's also present in the file, but it is not compiling without it.

Strange, I couldn't see any consistency between quoted and unquoted and escaped and un-escaped, I could find exceptions to all of them...

@matkoniecz
Copy link
Member

Though I would say that testing was also useful to detect potential problems with interface etc.

@westnordost
Copy link
Member

@smichel17 could you provide a screenshot? I am not sure the proposed text change doesn't break the layout. the new string is considerably longer.

@smichel17
Copy link
Member Author

smichel17 commented Aug 17, 2021

I was not sure either; I never checked :P. Anyway, I did now; on my device it looks like this:

Screenshots

d186d840-55af-468c-b268-287134791e72

ae9868b6-9231-4599-8b8e-e22f54612f4b

e191c176-9f99-487e-b34d-1da4be0e2b25

@smichel17
Copy link
Member Author

It is definitely a bit awkward with all that text, but it's functional, I think.

@westnordost
Copy link
Member

westnordost commented Aug 17, 2021 via email

@smichel17
Copy link
Member Author

smichel17 commented Aug 17, 2021

Summary: This is as good as the current released version and better than the recent update. edit: Actually, the first text needs to be shorter, just noticed it's clipped.

Mine is 5.1 inches. What device is 4.3 inches? I don't see any pre-made profiles in that size in Android Studio. But anyway, I tested on two emulated devices: A Nexus 4, which is 4.7" and 768x1280px (xhdpi), and a Nexus S, which is 4.0" and 480x800px (hdpi). For the record, this is small enough that even the intro screen clips.

Anyway, here are the results. Unfortunately I didn't keep very good track of which is which when taking screenshots, and this is already being more work than I wanted, so I'm just going to upload them in whichever order. 12 people in the group always clips with the keyboard open, even on the Nexus 4. If you close the keyboard, 12 people can be shown even on the smallest device.

Screenshots

Screenshot_1629204535

Screenshot_1629204589

Screenshot_1629205214

Screenshot_1629205463

Screenshot_1629205666

Screenshot_1629205725

@smichel17
Copy link
Member Author

Fixed text clipping, but I'm not sure how this will work with other translations. I'll try a German auto-translation.

@smichel17
Copy link
Member Author

smichel17 commented Aug 17, 2021

Okay, so, auf Deutsch, there is a little bit of clipping (two words) on the tiny device when the keyboard is open. When it is closed, there is no clipping and you can still select all 12 options. I think this is okay, especially considering it is an automatic translation, so perhaps a manual translation could be shorter. I don't know if there's a way to add a comment for translators, but it might be a good idea to instruct them to favor brevity over a perfect translation in this case.

If that's no good, I think it should be possible to combine the two sentences to something like, "How many people are mapping? Everyone should answer this." I don't like that phrasing as much, so I'd rather keep the English the same and use loose translations if necessary.

ScreenshotsScreenshot_1629208214

Screenshot_1629208224

Works in English. In a German auto-translation, there is still a little
clipping on a *tiny* device (Nexus S, 4", 480x800px, hdpi) when the
keyboard is open, but never when it is closed.

wrap_content is to prevent truncation.
android:layout_gravity is to re-center the text, which was starting low.
@smichel17
Copy link
Member Author

Removed an unnecessary change and improved the commit message; as long as the current amount of clipping (not very much, only on a tiny device and when the keyboard is open) is acceptable to you, this is ready to merge imo.

@westnordost
Copy link
Member

westnordost commented Aug 17, 2021

Top, that seems okay. As you say, it is as good as the current version.

4.3" was the display size of my old smartphone (Sony Xperia Z1 Compact) which I used until 2020. That display size was what was considered compact in 2014 and there were not many smaller phones smaller than that.
Nowadays (2019+), the smallest phone display size I could find was 5.8" but we got to keep in mind that it is perfectly okay to use the app with an older device. Sony Xperia Z1 Compact upgrades up to Android 5.1, so that device will still be supported even after choosing to not support Android 4.x anymore.

@westnordost westnordost merged commit a294678 into streetcomplete:master Aug 17, 2021
@peternewman
Copy link
Collaborator

Though I would say that testing was also useful to detect potential problems with interface etc.

Agreed, and #3167 should simplify that bit a lot.

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

6 participants