mozilla-mobile / fenix

⚠️ Fenix (Firefox for Android) moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
6.48k stars 1.27k forks source link

Bug 1815698 - Allow sponsored stories details to be spaced evenly vertically #28808

Closed Alexandru2909 closed 1 year ago

Alexandru2909 commented 1 year ago

Added SpaceEvenly arrangement to sponsored stories details to avoid having publisher text cut-off for normal fonts on some devices.

https://user-images.githubusercontent.com/35462038/217870656-c837af13-8c80-4b1e-8a72-045fa6d8dafc.mp4

Pull Request checklist

QA

To download an APK when reviewing a PR (after all CI tasks finished running):

  1. Click on Checks at the top of the PR page.
  2. Click on the firefoxci-taskcluster group on the left to expand all tasks.
  3. Click on the build-debug task.
  4. Click on View task in Taskcluster in the new DETAILS section.
  5. The APK links should be on the right side of the screen, named for each CPU architecture.

GitHub Automation

Used by GitHub Actions.

Alexandru2909 commented 1 year ago

I am approving this since it's soft freeze today. We have already started a discussion around the UX comment I had.

The proposed topic is around whether or not UX wants to persist the proportions within the element when it comes content sizes as a result of situation such as bigger text sizes. If so, then this will challenge how we have been using a fixed height for the container for most of our composables. Instead, we would want the container to wrap the contents.

Put another way, we're wondering if UX expects the proportions of the elements within the container to be maintained. In the screenshot, you will see that we’re using a fixed height container, but situations where the user uses bigger text, then the text content will need to be laid out more dynamically to accommodate the space it has to work with. Alternatively, the container size should be dynamic and just wrap elements within it while maintaining the expected paddings between all of the elements

Using dynamic height for the Pocket stories (Preview) compared to fixed height prior to landing this PR (Nightly)

https://user-images.githubusercontent.com/35462038/218117476-52d55b64-689c-47f3-824d-25ae55f6eec1.mp4

Mugurell commented 1 year ago

Thank you for the above video. Make sense that if we remove the hardcoded height than one item can be bigger than the others or many items can have different heights and then we end up with a staggered grid which seems like a worse result to me.