Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blaze: Show Blaze in general section #20314

Merged
merged 2 commits into from
Mar 13, 2023

Conversation

momo-ozawa
Copy link
Contributor

@momo-ozawa momo-ozawa commented Mar 13, 2023

Description

Shows the Blaze menu item in the general section. The general section is shown if the jetpack section isn't shown.

Note

The base branch for this PR will automatically update to release/21.9 once #20290 gets merged into release/21.9

How to test

  1. Create a test p2 site
  2. Change the site to be public
  3. On the app, go to the site menu tab in the My Site screen
  4. ✅ The Blaze menu item is displayed

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.

@peril-wordpress-mobile
Copy link

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

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 13, 2023

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr20314-10e18e7 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

Copy link
Contributor

@hassaanelgarem hassaanelgarem left a comment

Choose a reason for hiding this comment

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

Works as described 🚀
Left one small comment.

Comment on lines 878 to 888
CGSize iconSize = CGSizeMake(BlogDetailGridiconSize, BlogDetailGridiconSize);
UIImage *blazeIcon = [[UIImage imageNamed:@"icon-blaze"] resizedImage:iconSize interpolationQuality:kCGInterpolationHigh];
BlogDetailsRow *blazeRow = [[BlogDetailsRow alloc] initWithTitle:NSLocalizedString(@"Blaze", @"Noun. Links to a blog's Blaze screen.")
accessibilityIdentifier:@"Blaze Row"
image:[blazeIcon imageFlippedForRightToLeftLayoutDirection]
imageColor:nil
renderingMode:UIImageRenderingModeAlwaysOriginal
callback:^{
[weakSelf showBlaze];
}];
blazeRow.showsSelectionState = NO;
Copy link
Contributor

Choose a reason for hiding this comment

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

[Nit] What do you think of extracting this to a helper that creates a Blaze row to avoid code duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! 10e18e7

Base automatically changed from fix/blaze-pull-to-refresh to release/21.9 March 13, 2023 16:59
@wpmobilebot
Copy link
Contributor

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr20314-10e18e7 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@momo-ozawa
Copy link
Contributor Author

FYI @mokagio 🙇‍♀️

@momo-ozawa momo-ozawa merged commit 7c27e5a into release/21.9 Mar 13, 2023
@momo-ozawa momo-ozawa deleted the fix/show-blaze-in-general-section branch March 13, 2023 20:53
@mokagio mokagio mentioned this pull request Mar 15, 2023
3 tasks
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.

3 participants