Dynamic Dashboard Card Image UI Fix #22516

alpavanoglu · 2024-02-01T12:17:36Z

Fixes #22501

Testing Steps

Install & launch Jetpack App.
✅ Verify that the "New: Domains Management" card UI does not jump around.
Switch to another tab, and open My Site again.
✅ Verify that the "New: Domains Management" card UI looks correct.

Screen Recordings

Before

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-02-01.at.13.12.18.mp4

After

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-02-01.at.13.11.12.mp4

Regression Notes

Potential unintended areas of impact
N/A
What I did to test those areas of impact (or what existing automated tests I relied on)
N/A
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)

wpmobilebot · 2024-02-01T12:26:53Z

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr22516-cad7afd
	Version	24.1
	Bundle ID	org.wordpress.alpha
	Commit	`cad7afd`
	App Center Build	WPiOS - One-Offs #8677

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot · 2024-02-01T12:36:13Z

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr22516-cad7afd
	Version	24.1
	Bundle ID	com.jetpack.alpha
	Commit	`cad7afd`
	App Center Build	jetpack-installable-builds #7705

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

…o task/dynamic-dashboard-card-image-ui-fix

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/BlogDashboardDynamicCardCell.swift

RELEASE-NOTES.txt

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Dynamic/DynamicDashboardCard.swift

mokagio · 2024-02-04T23:04:08Z

I'm going to bump this to the next release because we'll be code freezing 24.2 today and this hasn't been approved yet.

If these changes cannot wait two weeks and it's important that it makes it into this release, let me know and we'll organize a new beta once ready.

alpavanoglu · 2024-02-05T10:09:26Z

Hey @mokagio
It would be great if we can target this release. I updated the PR and pinged the reviewers. Once it is ready, I'll rebase it to the release branch and leave the merging/not merging to you. If it is too late or anything, we can of course ship it with the next release.

salimbraksa · 2024-02-05T11:57:54Z

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Service/BlogDashboardRemoteEntity.swift

+                ) / CGFloat(
+                    height ?? 1
+                )
+            }


[Suggestion] I think if either width or height, is nil. I would expect the aspect ratio to be nil as well. Because it doesn't make sense to calculate the aspect ratio out of nil dimensions!

Makes sense!

salimbraksa · 2024-02-05T11:58:23Z

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Service/BlogDashboardRemoteEntity.swift

@@ -75,10 +76,24 @@ extension BlogDashboardRemoteEntity {
            let icon: String?
        }

+        struct ImageSize: Decodable, Hashable {
+            let width: Int?
+            let height: Int?


[Suggestion] I believe the type of these properties can be CGFloat even if the Backend returns them as Int.

I see your point, however I would rather match the backend type unless it is an enum if that's okay with you.

salimbraksa · 2024-02-05T12:01:07Z

@alpavanoglu I've dropped a few code suggestions but I confirm the UI glitch is resolved. Also, the unit tests are failing.

dangermattic · 2024-02-05T12:02:06Z

	1 Warning
⚠️	This PR is assigned to the milestone 24.2 ❄️. The due date for this milestone has already passed. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

salimbraksa

LGTM! 🚀

peril-wordpress-mobile · 2024-02-05T14:12:21Z

	Warnings
⚠️	This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

hassaanelgarem

🚀

mokagio · 2024-02-06T05:15:06Z

@alpavanoglu all good. Merging now.

alpavanoglu added 2 commits February 1, 2024 12:44

Set the height for whole of AsyncImage instead of the placeholder

4868467

Move frame constraint to placeholder

9fa761a

alpavanoglu added [Type] Task Domains Dashboard Card labels Feb 1, 2024

alpavanoglu added this to the 24.2 milestone Feb 1, 2024

Update Release Notes

772e1ad

alpavanoglu added 3 commits February 2, 2024 10:49

Temp

a0a9df5

Add aspect ratio logic

af32e5f

Merge branch 'trunk' of github.com:wordpress-mobile/WordPress-iOS int…

41c7adc

…o task/dynamic-dashboard-card-image-ui-fix

hassaanelgarem reviewed Feb 2, 2024

View reviewed changes

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/BlogDashboardDynamicCardCell.swift Outdated Show resolved Hide resolved

RELEASE-NOTES.txt Outdated Show resolved Hide resolved

salimbraksa reviewed Feb 3, 2024

View reviewed changes

WordPress/Classes/ViewRelated/Blog/Blog Dashboard/Cards/Dynamic/DynamicDashboardCard.swift Outdated Show resolved Hide resolved

salimbraksa self-requested a review February 3, 2024 20:21

mokagio modified the milestones: 24.2, 24.3 Feb 4, 2024

Address change requests

47eab11

salimbraksa reviewed Feb 5, 2024

View reviewed changes

salimbraksa self-requested a review February 5, 2024 12:01

salimbraksa assigned alpavanoglu Feb 5, 2024

salimbraksa modified the milestones: 24.3, 24.2 ❄️ Feb 5, 2024

alpavanoglu added 2 commits February 5, 2024 13:35

Fix tests & make width and height optional

6c03eef

Update dashboard-200-with-only-one-dynamic-card to match new response

cad7afd

salimbraksa approved these changes Feb 5, 2024

View reviewed changes

alpavanoglu changed the base branch from trunk to release/24.2 February 5, 2024 14:12

hassaanelgarem approved these changes Feb 5, 2024

View reviewed changes

mokagio merged commit e149425 into release/24.2 Feb 6, 2024
23 checks passed

mokagio deleted the task/dynamic-dashboard-card-image-ui-fix branch February 6, 2024 05:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic Dashboard Card Image UI Fix #22516

Dynamic Dashboard Card Image UI Fix #22516

alpavanoglu commented Feb 1, 2024

wpmobilebot commented Feb 1, 2024 •

edited

Loading

wpmobilebot commented Feb 1, 2024 •

edited

Loading

mokagio commented Feb 4, 2024

alpavanoglu commented Feb 5, 2024

salimbraksa Feb 5, 2024 •

edited

Loading

alpavanoglu Feb 5, 2024

salimbraksa Feb 5, 2024

alpavanoglu Feb 5, 2024

salimbraksa commented Feb 5, 2024

dangermattic commented Feb 5, 2024 •

edited

Loading

salimbraksa left a comment

peril-wordpress-mobile bot commented Feb 5, 2024

hassaanelgarem left a comment

mokagio commented Feb 6, 2024

Dynamic Dashboard Card Image UI Fix #22516

Dynamic Dashboard Card Image UI Fix #22516

Conversation

alpavanoglu commented Feb 1, 2024

Testing Steps

Screen Recordings

Before

After

Regression Notes

wpmobilebot commented Feb 1, 2024 • edited Loading

wpmobilebot commented Feb 1, 2024 • edited Loading

mokagio commented Feb 4, 2024

alpavanoglu commented Feb 5, 2024

salimbraksa Feb 5, 2024 • edited Loading

Choose a reason for hiding this comment

alpavanoglu Feb 5, 2024

Choose a reason for hiding this comment

salimbraksa Feb 5, 2024

Choose a reason for hiding this comment

alpavanoglu Feb 5, 2024

Choose a reason for hiding this comment

salimbraksa commented Feb 5, 2024

dangermattic commented Feb 5, 2024 • edited Loading

salimbraksa left a comment

Choose a reason for hiding this comment

peril-wordpress-mobile bot commented Feb 5, 2024

hassaanelgarem left a comment

Choose a reason for hiding this comment

mokagio commented Feb 6, 2024

wpmobilebot commented Feb 1, 2024 •

edited

Loading

wpmobilebot commented Feb 1, 2024 •

edited

Loading

salimbraksa Feb 5, 2024 •

edited

Loading

dangermattic commented Feb 5, 2024 •

edited

Loading