-
-
Notifications
You must be signed in to change notification settings - Fork 936
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
[android] New About screen #4439
Conversation
Really good changes !
|
Thanks for testing!
This blue MAPS.ME accent color wasn't updated during initial re-styling just by a mistake. I accidentally changed this part in this PR and it looked better to me. However, it looks like a distraction of the primary goal of this PR. I am extracting this change into #4444 and reverting here to avoid any speculations.
Fixed, thanks! This dangling "else" clause without brackets is really error-prone.
+1 I am also not satisfied with the color of the logo in dark mode, but this is the same color as on Splash screen. I thought about changing it to white in both places, but probably it would be better to address separately. |
Maybe we can create a different disposition for tablets in landscape mode ? |
I haven't started with landscape yet. |
Added OSM data version as @biodranik desperately wanted. |
35e93b5
to
a8baf8e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PT and PT-BR fix
f5be101
to
f79fe6a
Compare
|
@rtsisyk Can you please provide all other components (news, app strings,...) on weblate. It's easier for all and we can increase the quality. You can automagically merge the translations and the translators have an easier way to translate. |
Unfortunately, weblate doesn't support our current developer workflow for editing app strings. By moving to weblate we significantly complicate strings editing and adding for all contributors. We're looking for solutions for this problem that will make it easy to contribute and review strings for developers, and for Weblate users. |
Did you reach out to Weblate already? Maybe they could help you writing support for it. |
setupItem(R.id.report, isLandscape, root); | ||
|
||
final TextView supportUsView = root.findViewById(R.id.support_us); | ||
if (BuildConfig.FLAVOR.equals("google") && !TextUtils.isEmpty(mDonateUrl)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove this check and see if it works with the changed wording.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't get what you propose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose to always show support us. And use the hand with the heart icon for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I have changed "Support us" to "Support the project" as was requested above. I am not sure that new wording is really much different to get away with it. Let's keep it disabled for Google Play for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The icon is already "hand with the heart", no changes by this patch.
Yes, they do not support https://github.com/scelis/twine strings format that we're using (see data/strings/strings.txt for example). It is possible however to write a Weblate addon that will support it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Polish translations suggestions. Mostly fixes in conjugations because we are talking about "aplikacja" and the sentence about battery drain didn't sound natural.
@@ -0,0 +1,10 @@ | |||
<vector xmlns:android="http://schemas.android.com/apk/res/android" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logo is not visible in the dark theme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... the night (a.k.a dark) theme is implemented in some custom way which make it is hard to maintain. I will try to deal with styles and fix this issue and accent colors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setTint can help here, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setTint with what value? Styles are a bit messy and require refactoring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've solved this problem. Logo is now white in the dark mode.
f79fe6a
to
b255621
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions for French lines.
helpButton.setImageResource(R.drawable.ic_christmas_tree); | ||
helpButton.getDrawable().setTintList(null); | ||
helpButton.setImageResource(R.drawable.logo); | ||
// Keep this button colorful in normal theme. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is intentionally green to attract attention... Otherwise users would just give us *
star without even trying to read FAQ and/or report a bug... Anyway, we can experiment here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I bet that any user will tap an unknown button/icon at least once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is intentionally green to attract attention...
I bet that any user will tap an unknown button/icon at least once.
I also agree with @endim8 that this doesn't look good. Also the current question mark looks fine for me and it's much clearer that it's something related to "help/about".
For me the clean UI is one of the best things in OM. I understand that you try to get more users to read the about page, but I think many users really wouldn't like to have default UI of OM to be more "attention screaming" just to avoid some negative reviews on app stores.
I have two alternative ideas:
-
Maybe some mixed approach would be valuable? Like having this button with green color after first install/update of application and render it in gray color after it's been clicked once?
-
Or have possibility to disable this this button in options page? Some toggle that when it's ON then the 4 buttons change to 3 buttons (without the About Page), and the About Page appears just as another position when you click "hamburger menu" button. However, this might seem a bit like over-complicating setting page for not huge benefit. On the other hand there is already option like "hide zoom buttons" that affect what GUI element appear on app, so there is some precedence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@endim8 @mpawelski I agree with you in general and will try to improve the situation with this button in the next updates.
631b779
to
ea93b0f
Compare
@@ -664,8 +664,7 @@ | |||
|
|||
<activity | |||
android:name="app.organicmaps.help.HelpActivity" | |||
android:configChanges="orientation|screenLayout|screenSize" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It means that the activity will be recreated on screen rotation, and users will see a blank screen for a moment, more visible on older devices. Why is it mandatory for this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To enable the previous landscape mode!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least some TODO can be left here. Intentionally making the app slower does not fit well into Organic Maps "we are the fastest" approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What TODO? This change is needed to enable separate layout for portrait and landscape. Otherwise, screen rotation will use the previous layout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO to enable it back when layout will not be recreated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be recreated if we want a separate landscape mode.
@@ -0,0 +1,10 @@ | |||
<vector xmlns:android="http://schemas.android.com/apk/res/android" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setTint can help here, right?
@@ -28,6 +28,7 @@ | |||
<style name="MwmTextAppearance.Headline"> | |||
<item name="android:textSize">@dimen/text_size_headline</item> | |||
<item name="android:textColor">?android:textColorPrimary</item> | |||
<item name="android:fontFamily">@string/robotoMedium</item> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if this font is not installed on a device?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This string is overridden for old APIs.
@@ -48,15 +48,9 @@ public ViewHolder onCreateViewHolder(ViewGroup viewGroup, int viewType) | |||
public void onBindViewHolder(ViewHolder viewHolder, final int position) | |||
{ | |||
final MenuBottomSheetItem item = dataSet.get(position); | |||
viewHolder.getContainer().setOnClickListener((v) -> onMenuItemClick(item)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
По клику на лого можно приделать пасхалку.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Что практически предлагается сделать?
{ | ||
return format.parse(String.valueOf(dataVersion)); | ||
} | ||
catch (ParseException e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this catch necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ParseException is a checked exception.
Take out "[strings] Regenerated." commit |
Signed-off-by: Roman Tsisyk <roman@tsisyk.com>
Signed-off-by: Roman Tsisyk <roman@tsisyk.com>
ea93b0f
to
4ae4784
Compare
sv = Kartografiska data från OpenStreetMap från och med %@. Det är som Wikipedia där du kan lägga till eller redigera platser för alla användare över hela världen. | ||
sw = Takwimu za katuni kutoka kwa OpenStreetMap kama ya %@. Ni kama Wikipedia ambapo unaweza kuongeza au kuhariri maeneo kwa watumiaji wote ulimwenguni. | ||
th = ข้อมูลการทำแผนที่จาก OpenStreetMap ณ ปี %@มันเป็นเหมือนวิกิพีเดียที่คุณสามารถเพิ่มหรือแก้ไขสถานที่สำหรับผู้ใช้ทุกคนทั่วโลก | ||
tr = Harita verileri, %@ tarihine ait OpenStreetMap verilerinden alınmıştır. OpenStreetMap, Vikipedi'nin harita sürümü gibidir: dünyadaki herkes yer ekleyebilir veya düzenleyebilir. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tr = Harita verileri, %@ tarihine ait OpenStreetMap verilerinden alınmıştır. OpenStreetMap, Vikipedi'nin harita sürümü gibidir: dünyadaki tüm kullanıcılar için yer ekleyebilir veya düzenleyebilirsiniz.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the exact translation. But current is also OK.
I did my best to update translations... It would be nice to fit each item into one line, but this is not critical.
Edited: actualize screenshots