Skip to content

Today Widget - Tracks - #15456

Merged
Gio2018 merged 7 commits into
feature/ios-14-today-widgetfrom
feature/15158-today-widget-tracks
Dec 22, 2020
Merged

Today Widget - Tracks#15456
Gio2018 merged 7 commits into
feature/ios-14-today-widgetfrom
feature/15158-today-widget-tracks

Conversation

@Gio2018

@Gio2018 Gio2018 commented Dec 8, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #15158

To test:
Pre requisites:

  • Better to start from scratch and delete any previously installed version if you had tested widgets before.
  • Make sure you have iOS 14.3 installed on your device
  • In the developer settings, enable "WidgetKit Developer Mode"
  1. Build and run this version
  2. Install one or more widgets
  3. In mission control, select the tracks live view tool (Metrics -> Tracks -> Live view), and input wpios_today_home_extension_widget_updated as event name
  4. Wait for at least five minutes (sometimes it takes 8/9 minutes) and see the incoming tracks in the log
  5. Expand the tracks and verify that the info medium_widgets, small_widgets and total_widgets are consistent with that you installed

Known issue: two widgets of the same size and the same site, seem to be counted as one.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • 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.

@Gio2018 Gio2018 added this to the Pending milestone Dec 8, 2020
@Gio2018 Gio2018 self-assigned this Dec 8, 2020
houndci-bot
houndci-bot reviewed Dec 8, 2020
View reviewed changes
Comment thread WordPress/WordPressHomeWidgetToday/Tracks+TodayHomeWidget.swift Outdated

let previousCount = UserDefaults(suiteName: WPAppGroupName)?.object(forKey: WPHomeWidgetTodayCount) as? Int ?? 0
let newCount = widgetInfo.count

@houndci-bot houndci-bot Dec 8, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)

@peril-wordpress-mobile

peril-wordpress-mobile Bot commented Dec 8, 2020 •
edited
Loading

Copy link
Copy Markdown

You can trigger an installable build for these changes by visiting CircleCI here.

@Gio2018
Gio2018 marked this pull request as draft December 8, 2020 23:27
@peril-wordpress-mobile

peril-wordpress-mobile Bot commented Dec 8, 2020 •
edited
Loading

Copy link
Copy Markdown

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

houndci-bot
houndci-bot reviewed Dec 11, 2020
View reviewed changes
Comment thread WordPress/WordPressHomeWidgetToday/Tracks/Tracks+TodayHomeWidget.swift Outdated

let previousCount = UserDefaults(suiteName: WPAppGroupName)?.object(forKey: WPHomeWidgetTodayCount) as? Int ?? 0
let newCount = widgetInfo.count

@houndci-bot houndci-bot Dec 11, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)

@Gio2018
Gio2018 marked this pull request as ready for review December 17, 2020 01:39
@Gio2018

Gio2018 commented Dec 17, 2020

Copy link
Copy Markdown
Contributor Author

Converting it to ready for review as the results look more promising on iOS 14.3. Will still need some testing to confirm it works properly

@diegoreymendez

diegoreymendez commented Dec 17, 2020

Copy link
Copy Markdown
Contributor

I'm waiting to review this for now as there are CircleCI issues (and since I'm trying to resolve other unrelated issues). Let me know @Gio2018 if you still want me to go ahead anyway.

@Gio2018

Gio2018 commented Dec 17, 2020

Copy link
Copy Markdown
Contributor Author

I'm waiting to review this for now as there are CircleCI issues (and since I'm trying to resolve other unrelated issues). Let me know @Gio2018 if you still want me to go ahead anyway.

Hey @diegoreymendez , no particular rush on this one. The CircleCI issue is on the installable build that I triggered, and it's likely because it needs to be updated with the latest version of develop (not sure why the hound violation is still showing up there as it was already fixed)

frosty
frosty approved these changes Dec 18, 2020
View reviewed changes

@frosty frosty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems to be working as expected for me, with the exception of the known issue you mentioned 🙂 However, I feel that two identical widgets will be a fairly unlikely use case!

Comment thread WordPress/WordPressHomeWidgetToday/Tracks/Tracks+TodayHomeWidget.swift

UserDefaults(suiteName: WPAppGroupName)?.set(newCount, forKey: WPHomeWidgetTodayCount)

let properties = ["total_widgets": newCount,

@frosty frosty Dec 18, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could maybe add these keys to an enum?

@Gio2018
Gio2018 merged commit afd17d2 into feature/ios-14-today-widget Dec 22, 2020
@Gio2018
Gio2018 deleted the feature/15158-today-widget-tracks branch December 22, 2020 00:29
@staskus staskus mentioned this pull request Sep 11, 2023
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@diegoreymendez diegoreymendez Awaiting requested review from diegoreymendez
2 more reviewers
@houndci-bot houndci-bot houndci-bot left review comments
@frosty frosty frosty approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@Gio2018 Gio2018

Labels

None yet

Projects

None yet

Milestone

Pending

Development

Successfully merging this pull request may close these issues.

4 participants

@Gio2018 @diegoreymendez @frosty @houndci-bot