-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Improved styling the project settings dialog #5770
Conversation
@alyblenkin please take a look at this pr and let me know if you think we need to add something else. |
You're right, it's not ideal in landscape! I had an idea to try to save space with the settings button over the weekend...I'm going to mock it up and share it with you. |
@grzesiek2010 - thanks for restarting the build! It's working now :) Capturing a few notes from our conversation here:
|
I think it's much better than our small title because it creates an information hierarchy. It's also not taking up much more room because it's aligned with the cancel button. @lognaturel let me know if you disagree |
b693210
to
dd703c5
Compare
Just to double check. In the CircleCI apk it's still possible to click a main menu button and project icon at the same time.The PR #5759 is not included in the apk? |
2bdfb34
to
f170312
Compare
Now it should be included. |
@alyblenkin what do you think? That's the result of increasing the font size but also maybe the translation is not perfect in that case. |
@grzesiek2010 oh no...that certainly isn't working! I did not try using Kinyarwanda when testing it – this is good to know. We will likely need to bring the font back to the size before. Typically, this is fine because the X is on its own line, but we need to save space here. I'm assuming the buttons at the bottom are always cut off in Kinyarwanda because we didn't change anything? |
Yes. In all languages project names with a bigger font size are cut off and "Add project" e.g in Italian looks similar but it existed before and I think there's already an issue with it. |
Ok I've decreased the font size to what we had before. |
After testing we can merge this pr but in the future, we should maybe consider using a full-screen dialog in this case. @alyblenkin what do you think? Using a normal dialog it's not possible to accommodate everything and make everything look good with different translations. |
I completely agree! We were trying to avoid big changes with some of these design tweaks, but this work uncovered many issues with translations and how difficult it is to fit the content into the dialog in general. I'll add it to my list of UX/UI changes to discuss in our next release planning. |
7733dd4
to
0825849
Compare
Tested with Success! Verified on device with Android 12, 13 Verified cases:
|
Tested with Success! Verified on device with Android 8.1, 10 |
Closes #5732
Why is this the best possible solution? Were any other approaches considered?
This is what we have discussed in the issue and in this pr.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
Testing that the project's dialog works as expected will be enough since nothing else has been touched in this pr.
I've only moved the
x
button to the right side and changed the font size of the title.Do we need any specific form for testing your changes? If so, please attach one.
No.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No.
Before submitting this PR, please make sure you have:
./gradlew checkAll
and confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTest
locally.