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

Optimize UI #11435

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Optimize UI #11435

wants to merge 6 commits into from

Conversation

Hamza417
Copy link

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

Optimize the existing layout and how they work in the app with various accessibility improvements.

Before/After Screenshots/Screen Record

04
03
The list is optimized, the titles have better visibility and channel names have been italicized.

02
The grid list were incorrectly laid out in the app, the icons weren't evenly placed and have weird spacing, the same could be fixed easily by removing the fixed width and height constants and applying aspect ratio based resizing for the thumbnails. The rest of the changes were the same as list view.

01
The title has been emphasized, and comment usernames have better visibility now. The emphasis from the "X REPLIES" have been reduced to lower case so that it can be a little less loud.

Fixes the following issue(s)

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.

Due diligence

  - Also fixes the UI padding issue
  - Why use a constant height that too inside a constraint layout? Use a fixed aspect ratio instead.
  - Bold style for comment author
@github-actions github-actions bot added the size/medium PRs with less than 250 changed lines label Aug 13, 2024
@TobiGr TobiGr added GUI Issue is related to the graphical user interface accessibility Issue is related to accessibility labels Aug 13, 2024
Copy link
Member

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Thank you for the work.

app/src/main/res/layout-land/list_stream_card_item.xml Outdated Show resolved Hide resolved
@Hamza417 Hamza417 requested a review from TobiGr August 13, 2024 12:06
Copy link

sonarcloud bot commented Aug 13, 2024

@Angelk90
Copy link
Contributor

Angelk90 commented Aug 15, 2024

@Hamza417 : could you check if it is optimized correctly on tablets too? I noticed that there are some problems on tablets. you could also take a look at the stories. #6869

@Hamza417
Copy link
Author

@Hamza417 : could you check if it is optimized correctly on tablets too? I noticed that there are some problems on tablets. you could also take a look at the stories. #6869

Thanks for pointing out the exact issue, I'll see if these changes can be included as well before this PR gets merged.

Copy link

@acrodemocide acrodemocide left a comment

Choose a reason for hiding this comment

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

Code changes look good. I pulled down the APK and tested on my phone. I really love the new UI updates. Everything else on the APK appears to be working as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Issue is related to accessibility GUI Issue is related to the graphical user interface size/medium PRs with less than 250 changed lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inadequate accessibility of the app layouts and improper contrast between various views.
4 participants