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.

Sign up for GitHub

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

Jump to bottom

Promoted story cards fit images rather than constraining them #4684

Closed
BenHenning opened this issue Oct 31, 2022 · 3 comments · Fixed by #4686
Closed

Promoted story cards fit images rather than constraining them #4684

BenHenning opened this issue Oct 31, 2022 · 3 comments · Fixed by #4686
Assignees
@BenHenning
Milestone
0.10 Release Bloc...

Comments

@BenHenning
Copy link
Member

BenHenning commented Oct 31, 2022 •
edited
Loading

Describe the bug
Promoted story thumbnail cards do not consistently size thumbnail image sizes.

To Reproduce
Steps to reproduce the behavior:

  1. Open a tip-of-tree version of the app with production assets.
  2. Navigate to the home screen
  3. Ensure that "What is Addition?" shows up in the top recommended stories.
  4. Observe that the "What is Addition?" thumbnail is much larger than others.

Expected behavior
Promoted story card thumbnails should be consistently sized and not dependent on their physical sizes.

Demonstration
Screenshot:

image

Environment

  • Device/emulator being used: Nexus 5X
  • Android or SDK version (e.g. Android 5 or SDK 21): SDK 27
  • App version (you can get this through system app settings or via the admin controls menu in-app): developer build using e4e4de6.

Additional context
Note this requires production assets--the local developer assets don't seem to trigger the issue.
@BenHenning
Copy link
Member Author

BenHenning commented Oct 31, 2022 •
edited
Loading

91014af from #4587 seems suspicious since it changes promoted_story_card to wrap_content instead of match_parent.

@BenHenning BenHenning self-assigned this Oct 31, 2022
@BenHenning
Copy link
Member Author

NB: This is a really significant visual regression, so we should fix this before the next beta release.

@BenHenning BenHenning mentioned this issue Oct 31, 2022
Merged
6 tasks
BenHenning added a commit that referenced this issue Nov 1, 2022
@BenHenning
Fix #4684: Revert (most of) #4587 (#4686)
ac68f84 
## Explanation
Fixes #4684

This PR reverts #4587. I think the change of match_parent to
wraps_content caused the promoted story container to grow for larger
images, and this wasn't caught until we tried the changes with
production assets. I noticed other places in the PR that might have the
same issue, so rather than trying to make sense of everything in order
to properly fix it & add tests, I opted to revert the change since the
regression is beta-blocking (even though it wasn't a clean reversion and
required manual changes).

Note that the reversion includes keeping the changes to
recently_played_fragment since otherwise changes in
RecentlyPlayedFragmentTest fail, and the change seems unlikely to cause
any problems with respect to potential thumbnail sizing issues.

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
There's not much to show here--see the issue for context on the problem.
The post-reversion just reverts things back to their previous state
(i.e. no regression).
This was referenced Nov 4, 2022
Closed
Merged
Closed
@BenHenning
Copy link
Member Author

I can confirm this is fixed after testing the latest beta version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BenHenning BenHenning

Labels
None yet
Projects
None yet
Milestone
0.10 Release Blockers (Beta "Early Ac...
Development

Successfully merging a pull request may close this issue.

Fix #4684: Revert (most of) #4587 oppia/oppia-android
1 participant
@BenHenning