Skip to content

Implement new My Library UI#767

Merged
jsnshrmn merged 27 commits intoWikipediaLibrary:masterfrom
suecarmol:suecarmol/T285556-implement-new-my-library
Aug 3, 2021
Merged

Implement new My Library UI#767
jsnshrmn merged 27 commits intoWikipediaLibrary:masterfrom
suecarmol:suecarmol/T285556-implement-new-my-library

Conversation

@suecarmol
Copy link
Contributor

@suecarmol suecarmol commented Jul 27, 2021

Description

Implement the missing pieces of My Library UI after completing #762.

Rationale

This is part of TWL redesign efforts.

Phabricator Ticket

T285556

How Has This Been Tested?

Tested manually and created tests for the filters

Screenshots of your changes (if appropriate):

Screen Shot 2021-07-27 at 11 55 20

Types of changes

What types of changes does your code introduce? Add an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Minor change (fix a typo, add a translation tag, add section to README, etc.)

@suecarmol suecarmol requested a review from jsnshrmn July 30, 2021 23:51
@suecarmol suecarmol changed the title WIP - Implement new My Library UI Implement new My Library UI Jul 30, 2021
Copy link
Member

@jsnshrmn jsnshrmn left a comment

Choose a reason for hiding this comment

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

I'm giving this a pretty thorough lookover, so this review might be in progress for a bit. In the interest of more expedient feedback, I'll post comments as things come up.

The first thing I've noticed is that the language select filter has some odd behavior. On a fresh load/ hard reload (shift + F5) I find it almost impossible to select a language with a mouse click. Clicking left mouse also scrolls the list down and selects whichever language is beneath the cursor when the button is released. I'm able to (unreliably) select a language with the mouse once I've already fiddled with the control and there is some state.

@jsnshrmn
Copy link
Member

jsnshrmn commented Aug 2, 2021

Okay, I've manually tested emulating several devices (phone, tablet, desktop) and several languages (including hebrew, which is rtl) and it's functionally good IMO, other than the aforementioned issue with filtering languages. Moving on to a readthrough.

@suecarmol
Copy link
Contributor Author

The first thing I've noticed is that the language select filter has some odd behavior. On a fresh load/ hard reload (shift + F5) I find it almost impossible to select a language with a mouse click. Clicking left mouse also scrolls the list down and selects whichever language is beneath the cursor when the button is released. I'm able to (unreliably) select a language with the mouse once I've already fiddled with the control and there is some state.

I do not see this behavior in Chrome or Firefox for Mac. Do you have more details about how to reproduce this error?

@jsnshrmn
Copy link
Member

jsnshrmn commented Aug 2, 2021

It's very consistent for me on firefox 90 for ubuntu on this view, but I'm not seeing it on firefox 90 for windows or android on staging.

@jsnshrmn
Copy link
Member

jsnshrmn commented Aug 2, 2021

I will do some more testing to try to reproduce this in a clean Linux vm to verify that it's not something weird about my system.

@jsnshrmn
Copy link
Member

jsnshrmn commented Aug 3, 2021

We verified that this was a firefox 90.0 bug that's resolved in the current release, 90.02.

Copy link
Member

@jsnshrmn jsnshrmn left a comment

Choose a reason for hiding this comment

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

This looks great! I have one inline nitpick on localization strings, but other than that this is good to go IMO.

@suecarmol
Copy link
Contributor Author

I also fixed the : from the available collections and user collections templates

@jsnshrmn jsnshrmn merged commit 4395d6d into WikipediaLibrary:master Aug 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants