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

Standardize Headers: History #5169

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Standardize Headers: History #5169

wants to merge 1 commit into from

Conversation

l-olson1214
Copy link
Collaborator

Phabricator:
https://phabricator.wikimedia.org/T383826

Notes

Test Steps

  1. Enter several items in search to build up history
  2. Navigate to history
  3. Make sure scroll behavior works as expected
  4. Ensure profile is there and usable
  5. Check the same in landscape
  6. Repeat in iPad

Screenshots/Videos


let profileButtonConfig: WMFNavigationBarProfileButtonConfig?
if let dataStore {
profileButtonConfig = self.profileButtonConfig(target: self, action: #selector(userDidTapProfile), dataStore: dataStore, yirDataController: yirDataController, leadingBarButtonItem: nil, trailingBarButtonItem: nil)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code so far looks good - I noticed the Clear button disappeared though, unfortunately. I did a bit of digging and for mysterious reasons of the past that logic is buried in the superclass ArticleFetchedResultsViewController, but only this subclass takes advantage of it (due to it setting isDeleteAllVisible = true).

Can you move the Clear button-related logic out of ArticleFetchedResultsViewController and into HistoryViewController? Then on this line you can hopefully set the leadingBarButtonItem parameter to a clear button lazy property on self.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants