Lock Screen Widgets: Enable feature #21535

staskus · 2023-09-08T13:09:21Z

Enabling lock screen widgets by removing widget-related feature flags

To test:

All the functionality was already tested in other PRs. Confirm that lock widgets can be added after launching the app.

More testing steps: pc8eDl-14z-p2#comment-861

Regression Notes

Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)

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 · 2023-09-08T13:59:43Z

📲 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	pr21535-7a3e9c9
	Version	23.2
	Bundle ID	org.wordpress.alpha
	Commit	`7a3e9c9`
	App Center Build	WPiOS - One-Offs #7107

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

wpmobilebot · 2023-09-08T14:03:50Z

📲 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	pr21535-7a3e9c9
	Version	23.2
	Bundle ID	com.jetpack.alpha
	Commit	`7a3e9c9`
	App Center Build	jetpack-installable-builds #6151

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

guarani

Hey @staskus! The PR itself looks great. I did some testing to get a sense of whether this feels ready to ship and ran into some issues that I'm not sure how to diagnose.

Testing debug builds from Xcode

Widget asking me to add a site

Fresh install on iOS simulator after resetting the sim
Log into a testing WP.com account
Add widgets
Widgets say "Create or add a site to see today's stats" despite being logged into a WP.com account

Reproduced on iPhone simulator (iOS 16.4) and iPhone physical device (iOS 17 beta). Switching the site in the widget's site list didn't work either to resolve this.

Untitled.mp4

Unable to add widget

Fresh install on iOS device running iOS 17 beta
Log into a testing WP.com account
Tapped widget preview to add widget but it does nothing
Also note the widget previews are either blank or show a ghost loading state
Dragged widget preview over to widget area (widget or lock screen itself appears to crash, see below screen recording)
Re-opening the widget list, Jetpack is no longer in the list of apps
I restarted the device and I was able to add widgets

💡 Takeaway: iOS might "delist" apps from the list of available widgets and restarting the device seems to resolve this

RPReplay_Final1694558080.MP4

Widget showing zero stats

Fresh install on iOS device running iOS 17 beta
Log into a testing WP.com account
Note that there was a "leftover" widget on the lock screen despite this being a fresh install (seems like an iOS bug unrelated to our widgets)
Tapped widget preview to add the All-Time Views widget
The widget was showing zero
I tapped the widget, added the All-Time card, and noted there were 14 views
Widget now shows 14 views

Testing release builds via App Center

I felt like a lot of this could be related to debug builds so I downloaded the build from App Center (I vaguely remember Home screen widget issues on debug builds that weren't present on production builds). Oddly, no widgets appeared until I restarted the device (this again could be an iOS 17 beta issue).

Here things worked better but this issue of it showing zero for stats persisted. Here's a screen recording (same steps to reproduce as above "Widget showing zero stats"):

RPReplay_Final1694562402.MP4

What are your thoughts on this? I'm unsure if it's:

An iOS 17 beta issue (I don't have a non-beta device handy right now)
Reproducible by others (keen to hear if you can reproduce)
A temporary issue likely to go away after the user opens the app
An actual bug in the widget code (I haven't tried to debug this as I spent time testing to narrow things down to get steps to reproduce)

peril-wordpress-mobile · 2023-09-14T07:39:11Z

	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

…themselves After removing iOS13 today widget-related code, we also removed a commonly reused credential storing functionality. Re-adding it to StatsWidgetsStore

staskus · 2023-09-14T10:39:54Z

@guarani

Widget showing zero stats

I was able to reproduce it and diagnose the issue. It is a regression from #21521. StatsWidgetService reused credentials stored by TodayExtensionService to make network calls to update widget data by itself. I added this functionality in this commit 81ce636 so the issue should be resolved.

Widget asking me to add a site

I still haven't been able to reproduce it. I remember similar issues previously with an account that didn't have a default site set but nothing beyond that. Could you try to see if it's reproducible with some other account?

Unable to add widget

I never encountered such a thing. I will try to look for some explanations online.

guarani · 2023-09-14T16:24:35Z

I was able to reproduce it and diagnose the issue. It is a regression from #21521. StatsWidgetService reused credentials stored by TodayExtensionService to make network calls to update widget data by itself. I added this functionality in this commit 81ce636 so the issue should be resolved.

It's great to hear you could reproduce and find a fix ❤️ Regarding the other issues, they were not reproducible on Release builds, so perhaps they are non-issues. I think this can be merged if the zero stats issue is fixed, I'll review it today and hopefully, we can get this merged for Monday's beta 🤞

When using WidgetBundle and dealing with account state changes Apple recommends using reloadAllTimelines() https://developer.apple.com/documentation/widgetkit/keeping-a-widget-up-to-date

staskus · 2023-09-15T09:35:12Z

@guarani
I was testing widgets again.

One thing to note is that calling reloadTimelines does not mean that the widget will be immediately refreshed, that is for the system to decide. That is one thing I noticed when testing widgets, the time between calling reloadTimelines and widget reloading code can vary. Sometimes it happens immediately, sometimes it takes up to a minute.

I captured a sequence here:

00:00 Log out + reloadAllTimelines is called
🕥 00:27 widget reload happens to change it to loggedOut state
00:56 Log in + reloadAllTimelines is called
🕥 01:22 widget reload happens to change it to widgetData state
01:54 Log out + reloadAllTimelines is called
✅ 01:56 widget reload happens immediately to change it to loggedOut state

Reload.Timelines.Delay.720p.mov

I made little improvements based on Apple recommendations:

If your app uses WidgetBundle to support multiple widgets, you can use WidgetCenter to reload the timelines for all your widgets. For example, if your widgets require the user to sign in to an account but they have signed out, you can reload all the widgets by calling:
WidgetCenter.shared.reloadAllTimelines()

Since we're using WidgetBundle, I'm using reloadAllTimelines when we're reloading widgets due to account changing state.

- In rare cases with no default site, simply show the first site in the data - Restructured WidgetDataReader to only use a single method for snapshot and timeline for consistent data loading logic

staskus · 2023-09-15T12:56:44Z

I couldn't reproduce more issues myself but I just went through the code to see any other cases or code paths that could cause issues.

3d30474:

In rare cases with no default site, simply show the first site in the data
Restructured WidgetDataReader to only use a single method for snapshot and timeline for consistent data loading logic

guarani · 2023-09-15T14:51:54Z

Sorry I didn't review this yesterday!

Thanks for the changes you made since yesterday, I'll review this today. If it helps, I can merge this after reviewing to get this in the beta.

staskus · 2023-09-15T14:59:49Z

Sorry I didn't review this yesterday!
Thanks for the changes you made since yesterday, I'll review this today.

@guarani thanks! I'm still trying to figure out if my changes are causing UI test failures or are they flaky. They haven't passed since I merged trunk.

Update:
Good news, it was just fixed: #21582

If it helps, I can merge this after reviewing to get this in the beta.

Of course. If any issues come up, then we'll delay for 23.4. I will then only make a PR on Monday to merge 81ce636 to 23.3.

guarani

Thanks for all the work you put into this! I re-tested and it's working well!
I used a Debug build built from Xcode to add widgets, switch sites, and log into different accounts and the widgets worked well.

guarani · 2023-09-15T16:35:21Z

I'll merge this later today unless you do first 🙇

Remove widget-related feature flags

84c6081

staskus added [Type] Task iOS 16 Widgets labels Sep 8, 2023

staskus added this to the 23.3 milestone Sep 8, 2023

Update RELEASE-NOTES.txt

afc48fc

staskus marked this pull request as ready for review September 8, 2023 13:15

staskus requested a review from guarani September 12, 2023 06:48

guarani reviewed Sep 13, 2023

View reviewed changes

Merge branch 'trunk' into task/lock-screen-widgets-enable-feature-flag

7a8e6f7

Store Credentials to keychain so widgets could update information by …

81ce636

…themselves After removing iOS13 today widget-related code, we also removed a commonly reused credential storing functionality. Re-adding it to StatsWidgetsStore

staskus requested a review from guarani September 14, 2023 10:40

staskus added 2 commits September 15, 2023 10:05

Merge branch 'trunk' into task/lock-screen-widgets-enable-feature-flag

72e2dde

Reload all timelines after login/logout

92c232a

When using WidgetBundle and dealing with account state changes Apple recommends using reloadAllTimelines() https://developer.apple.com/documentation/widgetkit/keeping-a-widget-up-to-date

staskus added 2 commits September 15, 2023 15:54

Update WidgetDataReader to handle the case with no default site

3d30474

- In rare cases with no default site, simply show the first site in the data - Restructured WidgetDataReader to only use a single method for snapshot and timeline for consistent data loading logic

Merge branch 'trunk' into task/lock-screen-widgets-enable-feature-flag

5559644

Update WidgetDataReaderTests.swift

b7dacc2

Merge branch 'trunk' into task/lock-screen-widgets-enable-feature-flag

7a3e9c9

guarani approved these changes Sep 15, 2023

View reviewed changes

guarani merged commit 6ac3851 into trunk Sep 15, 2023

guarani deleted the task/lock-screen-widgets-enable-feature-flag branch September 15, 2023 19:04

staskus mentioned this pull request Oct 11, 2023

Add lock screen widgets to the app #19411

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lock Screen Widgets: Enable feature #21535

Lock Screen Widgets: Enable feature #21535

staskus commented Sep 8, 2023 •

edited

Loading

wpmobilebot commented Sep 8, 2023 •

edited

Loading

wpmobilebot commented Sep 8, 2023 •

edited

Loading

guarani left a comment •

edited

Loading

peril-wordpress-mobile bot commented Sep 14, 2023

staskus commented Sep 14, 2023

guarani commented Sep 14, 2023

staskus commented Sep 15, 2023

staskus commented Sep 15, 2023

guarani commented Sep 15, 2023

staskus commented Sep 15, 2023 •

edited

Loading

guarani left a comment

guarani commented Sep 15, 2023

Lock Screen Widgets: Enable feature #21535

Lock Screen Widgets: Enable feature #21535

Conversation

staskus commented Sep 8, 2023 • edited Loading

To test:

Regression Notes

wpmobilebot commented Sep 8, 2023 • edited Loading

wpmobilebot commented Sep 8, 2023 • edited Loading

guarani left a comment • edited Loading

Choose a reason for hiding this comment

Testing debug builds from Xcode

Widget asking me to add a site

Unable to add widget

Widget showing zero stats

Testing release builds via App Center

peril-wordpress-mobile bot commented Sep 14, 2023

staskus commented Sep 14, 2023

guarani commented Sep 14, 2023

staskus commented Sep 15, 2023

staskus commented Sep 15, 2023

guarani commented Sep 15, 2023

staskus commented Sep 15, 2023 • edited Loading

guarani left a comment

Choose a reason for hiding this comment

guarani commented Sep 15, 2023

staskus commented Sep 8, 2023 •

edited

Loading

wpmobilebot commented Sep 8, 2023 •

edited

Loading

wpmobilebot commented Sep 8, 2023 •

edited

Loading

guarani left a comment •

edited

Loading

staskus commented Sep 15, 2023 •

edited

Loading