Skip to content

Conversation

@dvdchr
Copy link
Contributor

@dvdchr dvdchr commented Oct 25, 2023

Fixes #21867

The new recommended tags card somehow doesn't refresh the cell's height on reloadData, which gets called after switching tabs. This PR fixes it with a workaround, by forcing the table view to update it's cell height by calling beginUpdates and followed with endUpdates immediately on the main queue.

To test

  • Launch the Jetpack app.
  • Go to Reader > Discover tab.
  • Refresh until you have a recommended tags card that spans two lines.
  • Tap the Following tab.
  • Tap to go back to Discover again.
  • 🔎 Verify that the recommended tags cell is displayed correctly.

Regression Notes

  1. Potential unintended areas of impact
    The Discover tab will now be forced to re-render its cells' height when reloaded.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manually tested the changes.

  3. What automated tests I added (or what prevented me from doing so)
    N/A.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes testing checklist:

  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@dvdchr dvdchr added this to the 23.5 ❄️ milestone Oct 25, 2023
@dvdchr dvdchr self-assigned this Oct 25, 2023
@dvdchr dvdchr requested a review from wargcm October 25, 2023 18:34
@wpmobilebot
Copy link
Contributor

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr21888-35e0101
Version23.5
Bundle IDorg.wordpress.alpha
Commit35e0101
App Center BuildWPiOS - One-Offs #7544
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr21888-35e0101
Version23.5
Bundle IDcom.jetpack.alpha
Commit35e0101
App Center Buildjetpack-installable-builds #6574
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

Copy link
Contributor

@wargcm wargcm left a comment

Choose a reason for hiding this comment

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

It appears this introduces a new issue:

large-you-might-like.mp4

@dvdchr dvdchr modified the milestones: 23.5 ❄️, 23.6 Oct 26, 2023
@dvdchr dvdchr changed the base branch from release/23.5 to trunk October 26, 2023 09:24
@dvdchr
Copy link
Contributor Author

dvdchr commented Oct 26, 2023

Ah good catch @wargcm. Since this is a workaround anyway, I will look into this and try to find a better solution. I'll target 23.6 instead.

@dvdchr dvdchr modified the milestones: 23.6, 23.7 Oct 26, 2023
@dvdchr dvdchr marked this pull request as draft November 1, 2023 09:21
@mokagio mokagio modified the milestones: 23.7, 23.8 Nov 13, 2023
@mokagio
Copy link
Contributor

mokagio commented Nov 13, 2023

I'm going to bump this to the next release because we'll be code freezing 23.7 today and this is still a draft.

@mokagio
Copy link
Contributor

mokagio commented Nov 27, 2023

I'm going to bump this to the next release because we'll be code freezing 23.8 today and this is still a draft.

@dvdchr just checking in on this, since it's been a draft with not activity for a few weeks. Is this still relevant?

@mokagio mokagio modified the milestones: 23.8, 23.9 Nov 27, 2023
@dvdchr dvdchr modified the milestones: 23.9, Someday Nov 30, 2023
@dvdchr
Copy link
Contributor Author

dvdchr commented Nov 30, 2023

Hi @mokagio, thanks for moving the milestone. This is an issue that I encountered back during beta, but the solution in this PR introduced side effects — and I didn't have time to work on a proper fix to address the issue.

I'll set the milestone to Someday for now, but our team is currently in Release Rotation; I plan to come back to this issue sometime during this sprint. If it works out, I'll update the milestone again, targeting 23.9. 🙂

This is necessary to get up to date with a CI configuration change that
requires running on Xcode 15.1.

See #22270
@mokagio
Copy link
Contributor

mokagio commented Jan 8, 2024

Hey @dvdchr 👋 You might notice a merge trunk commit here from me. That is to bring this branch up to date with the latest CI changes on trunk and ensure everything works on the new Xcode 15.1 CI stack.

Don't hesitate to ping me if there's any trouble.

@kean kean closed this Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Reader recommended tags' bottom border is clipped

6 participants