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

Via ferrata scale #480

Merged
merged 14 commits into from
Nov 1, 2023
Merged

Via ferrata scale #480

merged 14 commits into from
Nov 1, 2023

Conversation

mcliquid
Copy link

@mcliquid mcliquid commented Oct 30, 2023

Fix #479
This quest can be also used for sac_scale

Fix #479
This quest can be also used for sac_scale
Images are missing
Description is missing
@mcliquid mcliquid marked this pull request as ready for review October 30, 2023 20:00
@mcliquid
Copy link
Author

@Helium314 I would give it a go. I would implement the sac_scale mostly the same way - can I duplicate this code therefore?

@Helium314
Copy link
Owner

The quest looks mostly fine, but

@Helium314 I would give it a go. I would implement the sac_scale mostly the same way - can I duplicate this code therefore?

You mean a quest for sac_scale? Here I guess the a reasonable selection would be quite difficult.

@mcliquid
Copy link
Author

  • icon is in debug folder

sorry for that again - there is a super weird selection when importing assets. The next time it will work.

  • photo authors and licenses should be in authors.txt

hm, strange - they are: app/src/main/res/authors.txt
image

  • the quest disabled message should make some via ferratas are dangerous statment.

Added!

And here are some screenshots:
image
image
image
image

@Helium314
Copy link
Owner

hm, strange - they are: app/src/main/res/authors.txt

Sorry, I had checked the wrong authors.txt

@mcliquid
Copy link
Author

I've tested it on my physical phone: https://github.com/Helium314/SCEE/assets/3351668/20bc7548-fe57-4881-a1ea-0acf5f1de463
All good so far. All texts are displayed inside the image box (depends on text display size)
I've added the first image and made it brighter for more visibility.
Tested all 6 answers - all good.

Would be awesome if you could have a look again - from my side ready for merge.

@Helium314
Copy link
Owner

This is what it looks like on my phone:
screen
Text size appears larger because the screen is rather small.
For my taste the images are taking a little too much space, and potentially relevant details are hard to see. Especially grades 0 and 1 could be improved by zooming a little

My idea would be to use a slighly modified copy of cell_labeled_icon_select_smoothness with layout_constraintWidth_percent reduced a little and layout_constraintDimensionRatio adjusted.
Here (as an example)

<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout
    xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:layout_marginStart="0dp"
    android:padding="4dp"
    android:background="@drawable/image_select_cell">

    <ImageView
        android:id="@+id/imageView"
        android:layout_width="0dp"
        android:layout_height="0dp"
        android:scaleType="centerCrop"
        app:layout_constraintStart_toStartOf="parent"
        app:layout_constraintTop_toTopOf="parent"
        app:layout_constraintWidth_percent="0.50"
        app:layout_constraintDimensionRatio="15:10"
        android:adjustViewBounds="true"
        tools:src="@drawable/via_ferrata_scale_1"
        />

    <LinearLayout
        android:layout_width="0dp"
        android:layout_height="wrap_content"
        android:layout_marginStart="8dp"
        app:layout_constraintTop_toTopOf="parent"
        app:layout_constraintStart_toEndOf="@id/imageView"
        app:layout_constraintEnd_toEndOf="parent"
        android:orientation="vertical">

        <TextView
            android:id="@+id/textView"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            style="@style/TitleNextToImage"
            tools:text="@string/quest_viaFerrataScale_one" />

        <TextView
            android:id="@+id/descriptionView"
            style="@style/DescriptionNextToImage"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            tools:text="@string/quest_viaFerrataScale_one_description" />

    </LinearLayout>

</androidx.constraintlayout.widget.ConstraintLayout>

@mcliquid
Copy link
Author

mcliquid commented Nov 1, 2023

I've optimized all images to a square format, because I think this shows the most details of the pictures. What do you think:
(Simulator is a Pixel 7 Pro - big screen though)
image

@Helium314
Copy link
Owner

Makes the quest form pretty huge, but I agree it's a clear improvement.

@mcliquid
Copy link
Author

mcliquid commented Nov 1, 2023

It's hard to test on multiple devices, but we could decrease the size of the images from 50% to 40ish%?

@Helium314
Copy link
Owner

I guess 50% is still fine, as width smaller images it's harder to see details. I'll have a look at 40% and 50% versions on my phone.

@Helium314
Copy link
Owner

Actually both variants look good to me, it up to you to choose 40, 50, or anything in between.

@mcliquid
Copy link
Author

mcliquid commented Nov 1, 2023

Than let's go with 40% as I think the quest is not so long in total.

@Helium314
Copy link
Owner

Some details regarding the strings:

  • I'm not familiar with the English terminology, but search results suggest that it should always be "via ferrata", not just "ferrata".
  • via_ferrata's -> via ferratas
  • I think it's ok to add the text from quest_viaFerrataScale_hint to the disabled message
  • User of / in the descriptions does not look so good, I think it should be replaced e.g. by or or ,. In the titles it's ok.

@mcliquid
Copy link
Author

mcliquid commented Nov 1, 2023

  • I'm not familiar with the English terminology, but search results suggest that it should always be "via ferrata", not just "ferrata".

A valid occurence is for the "Ferrata Set", as it's not a "Via Ferrata Set" also in the official guidelines.

All other things are considered.

@Helium314 Helium314 merged commit 9b036b1 into Helium314:modified Nov 1, 2023
@mcliquid mcliquid deleted the via_ferrata_scale branch November 1, 2023 11:41
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.

New Quest: What is the via_ferrata_scale on this path?
2 participants