Closed Bug 1109641 Opened 5 years ago Closed 5 years ago
Disable the back/forward buttons when there is nothing to do
Disable the back/forward buttons when there is nothing to do.
Buttons are private so can't be disabled from outside the toolbar class/object. Should we make them public? Or should we create 2 separate methods in toolbar for disabling each button? Also when should these methods be triggered? Should we use WKNavigationDelegate and may be trigger these methods from - webView:didCommitNavigation: - here we check if these is anything in the list, if there isn't, then we disable?
Brainstorming: It doesn't look like we have any code that tracks the WKWebView navigation progress yet. I assume the BrowserViewController could do that. The BrowserToolbar should probably have a method that allows the controller to update it's state after a page loads (or fails to load). We'll need to update the URL as well, for example. The state we pass into that BrowserToolbar method should have enough information for the toolbar to set the state of the buttons.
Using KVO you can watch the WKWebView's canGoBack and canGoForward properties and update the UI accordingly.
I like the KVO idea, it's similar to how the progress bar is being tracked. In addition to the KVO, you have to take into account when switching tabs now. So update the UI accordingly whenever a new tab is switched to since you are dealing with a new webView. This I would bet be made in -- didSelectedTabChange
Somebody already submitted a PR for it - https://github.com/mozilla/firefox-ios/pull/53. Haven't had time to look at it though.
apbendi has a PR for this. We can't use KVO directly as suggested in comment 3 since canGoBack/canGoForward are not KVO-compliant properties. I suggest updating the back/forward state in webViewDidStartLoad, when a page begins loading.
Attachment #8544287 - Flags: feedback+
Comment on attachment 8544287 [details] [review] Pull request Merged awhile ago. Looks like I forgot to update this bug.
Attachment #8544287 - Flags: feedback+ → review+
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.