My Site Dashboard: Refactor drafts and scheduled posts cells

[hassaanelgarem]

Part of #18308

Description

• This PR makes use of a new cell DashboardPostsListCardCell to display drafts and schedules posts cards.
• "Create Next" and "Creat First" cards are still using the old DashboardPostCardCell
• DashboardPostsListCardCell is made up of logic extracted from DashboardPostCardCell and PostsCardViewController

Next PR(s) Will Include

• Create new cell for displaying "Create Next" and "Creat First" cards
• Handle cases where cards should be refreshed (All drafts deleted for example)
• Remove all unnecessary code

Testing Instructions

As with previous PRs, no testing intructions as of now.

Regression Notes

1. Potential unintended areas of impact
   N/A

2. What I did to test those areas of impact (or what existing automated tests I relied on)
   N/A

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.

[wpmobilebot]

You can test the changes in WordPress from this Pull Request by:

• Clicking here or scanning the QR code below
• Then installing the build number pr18340-84faa36 from App Center on your iPhone

The .ipa file can also be downloaded directly here.
If you need access to App Center, please ask a maintainer to add you.

[wpmobilebot]

You can test the changes in Jetpack from this Pull Request by:

• Clicking here or scanning the QR code below
• Then installing the build number pr18340-84faa36 from App Center on your iPhone

The .ipa file can also be downloaded directly here.
If you need access to App Center, please ask a maintainer to add you.

[leandroalonso]

@hassaanelgarem left a few suggestions.

Also, to add to your list of requirements for the next tasks:

1. Remove a card when all its posts are removed (scheduled or draft)
2. Add a new card when needed (eg.: scheduling a post should make the scheduled cards appear. Or if the scheduled is appearing and a new draft is created the draft card should appear)

Looking forward to removing code in the next PRs. :)

[leandroalonso]

You can remove that. :D

[hassaanelgarem]

Done in 73ce726 😅

[leandroalonso]

Is your plan to leave the cardType here?

I ask that because it seems a little bit odd. If I'm new to this code I would expect a 1:1 to map on DashboardCard for enum ~> cell. In this case, here we changed the protocol signature for this specific case (a specific cell needs to know which card it should represent).

Do you think we could split the different types of posts cards into different cells? We could use composition or inheritance. This way we would have DashboardDraftCardCell, DashboardScheduledCardCell, DashboardFirstPostCardCell and DashboardNextPostCardCell.

Also, by doing that you can get rid of presentedPostStatuson the cell.

While maybe this means more lines of code, I think it's clearer. Thoughts?

[hassaanelgarem]

That's a great point! I went with that initially cause I felt there's no need to create multiple cell classes. However, now I feel that would be much clearer.

I've changed this in 84faa36

I used inheritence and kept DashboardPostsListCardCell almost the same. The new subclasses are only responsible for passing the card type to DashboardPostsListCardCell instead of including it in the protocol. This is to avoid making variables under DashboardPostsListCardCell public.

Also, by doing that you can get rid of presentedPostStatuson the cell.

I don't think we can do that. presentedPostStatus is needed by BlogDashboardViewController not the cell. It's needed because that VC needs to conform to EditorAnalyticsProperties.

[momo-ozawa]

I've changed this in 84faa36

1:1 mapping between the DashboardCard enum to a cell looks great!

[leandroalonso]

I did a few tests with an account with 5 sites, and the PostsCardViewModel is being retained on a few occasions:

I think this is something we might want to address because each of those view models has an NSFetchedResultsController and its observing posts.

I believe this doesn't affect many users but it's a good improvement anyway. :)

[hassaanelgarem]

I took a look at this and it seems that the max instances of PostsCardViewModel retained at anytime is 5.
The collection view always keeps an extra DashboardPostsListCardCell, and each cell holds one view model. So the max here is 3 view models.

And there's always 2 instances of PostsCardViewController and each hold one view model as well. These however should be removed in the following PR.

[hassaanelgarem]

This should be improved in #18351 since PostsCardViewController is completely removed.
However, since we split DashboardPostsListCardCell to two classes, now the collection view retains an extra cell for each. So the max is now 4 view models.

[leandroalonso]

Ok! I'll check that again in #18351 and think if there's anything we might be able to do, thanks for the explanation!

[hassaanelgarem]

Also, to add to your list of requirements for the next tasks:

These were really helpful!