Fix #8526: Fix the issue of the share button disappearing

[nik3212]

Summary of Changes

This pull request fixes brave/brave-browser#36101

Submitter Checklist:

• [ ] Unit Tests are updated to cover new or changed functionality
• [x] User-facing strings use NSLocalizableString()
• [x] New or updated UI has been tested across:
  • [x] Light & dark mode
  • [x] Different size classes (iPhone, landscape, iPad)
  • [x] Different dynamic type sizes

Test Plan:

Screenshots:

https://github.com/brave/brave-ios/assets/17319991/738cdd9d-a62f-4082-a073-aa1d3dd445fa

Reviewer Checklist:

• [ ] Issues include necessary QA labels:
  • QA/(Yes|No)
  • bug / enhancement
• [ ] Necessary security reviews have taken place.
• [ ] Adequate unit test coverage exists to prevent regressions.
• [ ] Adequate test plan exists for QA to validate (if applicable).
• [ ] Issue and pull request is assigned to a milestone (should happen at merge time).

[soner-yuksel]

@nik3212 thank you for the contribution, good solution. Only one simple comment to for changes. https://github.com/brave/brave-ios/pull/8572#discussion_r1430403074

[nik3212]

If this piece of code is moved inside observeValue, it doesn't seem to work correctly. The share button doesn't disappear. I believe it should be executed immediately once a navigation is initiated.

In this case, when a user navigates back, the toolbar isn't affected because canGoForward is not always executed. What do you think?

https://github.com/brave/brave-ios/assets/17319991/82055f9a-0e85-462c-a5de-e20750d39b47

[soner-yuksel]

If this piece of code is moved inside observeValue, it doesn't seem to work correctly. The share button doesn't disappear. I believe it should be executed immediately once a navigation is initiated.

In this case, when a user navigates back, the toolbar isn't affected because canGoForward is not always executed. What do you think?

@nik3212 this is where the change should occur https://github.com/brave/brave-ios/blob/72bdc51a80a1df6323bfff67f47672be57bba61d/Sources/Brave/Frontend/Browser/BrowserViewController.swift#L1804C25-L1804C25

Here it is while working with code

https://github.com/brave/brave-ios/assets/6643505/d42d20d5-96aa-40a9-996d-eee214407e43

[nik3212]

@soner-yuksel, yes, I definitely agree with you. In this particular case, it works as expected. However, when a user takes more than one step back, it causes an error with updating the forward button. You can observe this behavior in this video:

https://github.com/brave/brave-ios/assets/17319991/948ebc3d-3b96-4971-a275-90835261dad3

If this piece of code is placed inside webView(_ webView: WKWebView, didCommit navigation: WKNavigation!), it will update the button state correctly:

https://github.com/brave/brave-ios/assets/17319991/47f04ef6-0f4e-4077-95ab-f4e8df9cdfe5

[soner-yuksel]

@soner-yuksel, yes, I definitely agree with you. In this particular case, it works as expected. However, when a user takes more than one step back, it causes an error with updating the forward button. You can observe this behavior in this video:

If this piece of code is placed inside webView(_ webView: WKWebView, didCommit navigation: WKNavigation!), it will update the button state correctly:

@nik3212 good catch, I want to refactor this part it is getting confusing related with updateForwardStatus but we should merge your fix. thanks again for the contribution. Please continue contributing and also creating issues for requests and problems you are seeing while using the app.