Fix incorrect Navigation Bar Appearance in Reader Detail screen by salimbraksa · Pull Request #20363 · wordpress-mobile/WordPress-iOS

salimbraksa · 2023-03-18T19:22:23Z

Fixes #20362

Description

This PR resolves a regression bug caused in this PR:

Change navigation bar and top tabs background color in Reader and Notifications #20278

The aforementioned PR changed the Reader's navigation bar appearance from transparent to opaque. The new navigation bar appearance is inherited by all view controllers that get pushed to the same navigation controller. But one view controller should keep the transparent navigation bar appearance. And that is the ReaderDetailViewController.

This PR resolves that issue by refactoring the navigation bar appearance logic in ReaderDetailViewController. So instead of setting navigationBar.standardAppearance, I used navigationItem.standardAppearance instead.

Notes

I recommend to review each commit individually and in this order:

Screenshots

Before	After

Test Instructions

Install WordPress / Jetpack and login.
Enable Light Mode.
Head to a Post with a Featured Image:
- Deep Link To: https://lostinamberland.wordpress.com/2023/03/18/way-too-far/
Verify the navigation bar appearance is transparent.

Regression Notes

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

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-03-18T19:45:00Z

📲 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	pr20363-78b9db9
	Version	22.0
	Bundle ID	com.jetpack.alpha
	Commit	`78b9db9`
	App Center Build	jetpack-installable-builds #4291

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

wpmobilebot · 2023-03-18T20:52:06Z

📲 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	pr20363-78b9db9
	Version	22.0
	Bundle ID	org.wordpress.alpha
	Commit	`78b9db9`
	App Center Build	WPiOS - One-Offs #5266

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

mokagio · 2023-03-20T02:31:10Z

I'm about to start the 22.0 code freeze. This PR is not ready to land into trunk, but it seems worth leaving under the 22.0 milestone and update to point to release/22.0 once the branch is ready given it's a UI fix.

salimbraksa · 2023-03-20T15:58:00Z

+            return navigationBar?.tintColor
+        }
+        set(newValue) {
+            // Ideally, we should be setting the navigationItem's tint color, but for some reason that didn't work.


This is resolved in 4543fed

salimbraksa · 2023-03-20T15:59:01Z

+    /// Flag indicating whether the navigation bar appearance needs restoration when the view disappears. Default is `false`.
+    ///
+    /// Flag changes to `true` when this view changes the navigation bar tint color. In other words, when the `navBarTintColor` setter is called.
+    private var navigationBarAppearanceNeedsRestoration = false


This is removed in 4543fed

salimbraksa · 2023-03-20T15:59:09Z

+    /// The original navigation bar tint color.
+    ///
+    /// This property is used to restore the navigation bar tint color to its original value.
+    private var previousNavBarTintColor: UIColor?


This is removed in 4543fed

salimbraksa · 2023-03-20T16:00:33Z

+        navigationItem.scrollEdgeAppearance = appearance
+        if #available(iOS 15.0, *) {
+            navigationItem.compactScrollEdgeAppearance = appearance
+        }


Setting the navigationItem's appearance ( instead of navigationBar ) is the most important change in the PR.

…ppear white in posts with featured image

peril-wordpress-mobile · 2023-03-20T16:54: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

alpavanoglu · 2023-03-21T15:20:37Z

+            guard let self = self else {
+                return
+            }


Tidbit:
Now we can do

guard let self else { return }

alpavanoglu · 2023-03-21T15:22:19Z


-    public func deviceDidRotate() {
-        guard !useCompatibilityMode else {
+    public func load(completion: @escaping () -> Void) {


This function handles a bit too much of action here. But no rush for fixing it since we need it in the hotfix

alpavanoglu

Test Succeeded ✅

salimbraksa added [Type] Bug Reader Jetpack App labels Mar 18, 2023

salimbraksa added this to the 22.0 milestone Mar 18, 2023

salimbraksa self-assigned this Mar 18, 2023

salimbraksa commented Mar 20, 2023

View reviewed changes

salimbraksa force-pushed the task/20362-reader-nav-bar-appearance branch from 4543fed to 83c68db Compare March 20, 2023 16:05

salimbraksa marked this pull request as ready for review March 20, 2023 16:34

salimbraksa added 3 commits March 20, 2023 16:54

Resolve a regression bug that was causing the nav bar appearance to a…

a7c56d2

…ppear white in posts with featured image

Reorganize methods and properties

1daff75

Update the navigation bar tintColor using its navigation item

78b9db9

salimbraksa force-pushed the task/20362-reader-nav-bar-appearance branch from 83c68db to 78b9db9 Compare March 20, 2023 16:54

salimbraksa requested a review from alpavanoglu March 20, 2023 16:54

salimbraksa changed the base branch from trunk to release/22.0 March 20, 2023 16:54

alpavanoglu reviewed Mar 21, 2023

View reviewed changes

alpavanoglu approved these changes Mar 21, 2023

View reviewed changes

salimbraksa merged commit 2fed749 into release/22.0 Mar 22, 2023

salimbraksa deleted the task/20362-reader-nav-bar-appearance branch March 22, 2023 10:15

mokagio mentioned this pull request Mar 29, 2023

Incorrect Navigation Bar Appearance in Reader Detail screen #20362

Closed

This was referenced Apr 6, 2023

Reader: Crash on iPad after tapping more/share button in the details view #20487

Closed

Fix Reader Crash on iPad after tapping more/share button in the details view #20489

Closed

salimbraksa mentioned this pull request Apr 7, 2023

Fix: App crashes when tapping more and share button in the Reader Details screen #20490

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix incorrect Navigation Bar Appearance in Reader Detail screen#20363

Fix incorrect Navigation Bar Appearance in Reader Detail screen#20363
salimbraksa merged 3 commits into
release/22.0from
task/20362-reader-nav-bar-appearance

salimbraksa commented Mar 18, 2023 •

edited

Loading

Uh oh!

wpmobilebot commented Mar 18, 2023 •

edited

Loading

Uh oh!

wpmobilebot commented Mar 18, 2023 •

edited

Loading

Uh oh!

mokagio commented Mar 20, 2023

Uh oh!

salimbraksa Mar 20, 2023

Uh oh!

salimbraksa Mar 20, 2023

Uh oh!

salimbraksa Mar 20, 2023

Uh oh!

salimbraksa Mar 20, 2023

Uh oh!

peril-wordpress-mobile Bot commented Mar 20, 2023 •

edited

Loading

Uh oh!

alpavanoglu Mar 21, 2023

Uh oh!

alpavanoglu Mar 21, 2023

Uh oh!

alpavanoglu left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

salimbraksa commented Mar 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes

Screenshots

Test Instructions

Regression Notes

PR submission checklist:

Uh oh!

wpmobilebot commented Mar 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Mar 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mokagio commented Mar 20, 2023

Uh oh!

salimbraksa Mar 20, 2023

Choose a reason for hiding this comment

Uh oh!

salimbraksa Mar 20, 2023

Choose a reason for hiding this comment

Uh oh!

salimbraksa Mar 20, 2023

Choose a reason for hiding this comment

Uh oh!

salimbraksa Mar 20, 2023

Choose a reason for hiding this comment

Uh oh!

peril-wordpress-mobile Bot commented Mar 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alpavanoglu Mar 21, 2023

Choose a reason for hiding this comment

Uh oh!

alpavanoglu Mar 21, 2023

Choose a reason for hiding this comment

Uh oh!

alpavanoglu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

salimbraksa commented Mar 18, 2023 •

edited

Loading

wpmobilebot commented Mar 18, 2023 •

edited

Loading

wpmobilebot commented Mar 18, 2023 •

edited

Loading

peril-wordpress-mobile Bot commented Mar 20, 2023 •

edited

Loading