Improve usability of search languages bar (1 of 2)

[staykids]

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

Notes

This is part 1 of 2 in improving the usability of the language bar in the search controller. The specific task elements addressed by this PR are:

1. Allow all of user's preferred languages to be displayed in search language bar (instead of the previous limitation of 3)
2. Increase the size of the "more languages" gradient overlay to match design
3. Updates "More" button title color to persistently be the theme's primary link color.
4. Update displayed language name to match its display in the preferred "Wikipedia languages" view controller. That is, the name of a language in the search language bar should match that language's displayed header text in the "Wikipedia languages" view controller.

The next PR will address the design updates.

Test Steps

Confirm there is behavioral parity with the existing search bar, excepting the changes noted above. Some examples include:

1. Confirm adding and removing languages results in the proper number of language bar buttons displayed.
2. Confirm tapping a search bar button and making a search functions as it did before.
3. Confirm it renders as expected in all themes.

Screenshot

[staykids]

Hmm, I noticed something strange here and I'm not sure what's up. When switching the device's language to an RTL language, the UIScrollView in the language bar... stops scrolling 🤔. Its child views (the SearchLanguageButtons) still seem to be receiving touches though...

[tonisevener]

Hmm, I noticed something strange here and I'm not sure what's up. When switching the device's language to an RTL language, the UIScrollView in the language bar... stops scrolling 🤔. Its child views (the SearchLanguageButtons) still seem to be receiving touches though...

I took a small peek and it does seem related to this constraint - I'm not sure why it causes problems but when I replace with stackView.leadingAnchor.constraint(equalTo: scrollView.leadingAnchor, constant: 8).isActive = true it scrolls.

[dempseyatgithub]

One thing I'm concerned about is the size of tap targets for languages with short names.

That won't be an issue when the square icons are in place. If we think those will go in, then that solves the issue. Otherwise we might need a minimum width for a language (which will make the spacing look weird for short language names.)

[dempseyatgithub]

I'm not sure the coding style. Should the reason string be moved over as well? In this case it's not necessarily clear there should be a selection by the time viewWillAppear() is called, so it might be good to move it over as well.

[staykids]

This is another case where I was just converting over what existed previously. I'm just going to remove this assertion. Updated!

[dempseyatgithub]

Hmm, I noticed something strange here and I'm not sure what's up. When switching the device's language to an RTL language, the UIScrollView in the language bar... stops scrolling 🤔. Its child views (the SearchLanguageButtons) still seem to be receiving touches though...

I took a small peek and it does seem related to this constraint - I'm not sure why it causes problems but when I replace with stackView.leadingAnchor.constraint(equalTo: scrollView.leadingAnchor, constant: 8).isActive = true it scrolls.

I think UIScrollView is supposed to do the correct thing if you just set the anchors for each side equal to each other.

The constant of 8 should probably be a content inset of 8 instead of part of the constraint, since conceptually, the stack view should be the entire content area, just offset by 8 points.

[dempseyatgithub]

One other thing I noticed is that the selection indicator bar is positioned in the exact same area as the scrolling indicator, which leads to odd visual interactions when scrolling.

[tonisevener]

@staykids One other thing I wanted to mention - this would be new functionality but I suggest we automatically scroll to the highlighted language after it's selected. Now that the list can get long it's easy to get lost if it's highlighting off screen. I tried this in a suggestions branch here. I also noticed a couple of other layout things:

Here I don't think the scrollView contentInset is adjusting based on RTL so the fade creeps in even though this is scrolled as much as possible:

And seems like a safe area alignment is needed somewhere here (this is also scrolled as much as possible):

[staykids]

I took a small peek and it does seem related to this constraint - I'm not sure why it causes problems but when I replace with stackView.leadingAnchor.constraint(equalTo: scrollView.leadingAnchor, constant: 8).isActive = true it scrolls.

@tonisevener Strange – I was wondering why that specific constraint previously existed in the XIB. Thanks for looking at and discovering this!

I think UIScrollView is supposed to do the correct thing if you just set the anchors for each side equal to each other.

The constant of 8 should probably be a content inset of 8 instead of part of the constraint, since conceptually, the stack view should be the entire content area, just offset by 8 points.

@dempseyatgithub Good point – I was just recreating the existing XIB UIStackView with its existing constraints programmatically, but it seems sensible in this case to just pin the edge and use content inset. This one will be addressed in my part 2 follow-up PR.

[staykids]

One thing I'm concerned about is the size of tap targets for languages with short names.

@dempseyatgithub I agree, the minimized tappable width here with language name alone is not ideal. But yeah, the extra tappable space afforded by the language code square in that forthcoming PR helps. After merged, we'll put the Phab task in design QA for review.

[staykids]

One other thing I noticed is that the selection indicator bar is positioned in the exact same area as the scrolling indicator, which leads to odd visual interactions when scrolling.

@dempseyatgithub I noticed this as well and agree, it's uncomfortable, but is the previously existing behavior/design of the bar. I'll include a screenshot of this scroll indicator when the task is moved to design QA.

[staykids]

@staykids One other thing I wanted to mention
Now that the list can get long it's easy to get lost if it's highlighting off screen. I tried this in a suggestions branch
couple of other layout things:
Here I don't think the scrollView contentInset is adjusting based on RTL so the fade creeps in even though this is scrolled as
And seems like a safe area alignment is needed somewhere here (this is also scrolled as much as possible):

@tonisevener Thanks for catching these and the suggestion branch! I'm already working on some RTL updates in the 2nd PR for this task, so I think it's best I address these in that one.

[staykids]

I believe this is ready for another look – let me know if I missed any of your comments, but I think the addressable ones I mention I'll handle in the part 2 PR I'm working on for this task.

[dempseyatgithub]

Since the remainder of the changes are coming in the next PR this looks fine to merge.