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

(5P) External Media: Remove existing Google Photos filters in favor of date-range dropdown #15880

Closed
WunderBart opened this issue May 21, 2020 · 34 comments · Fixed by #16190
Closed
Assignees
Labels
[Extension] External Media Extend all block editor media tools to support external providers [Status] Needs Design [Type] Bug When a feature is broken and / or not performing as intended

Comments

@WunderBart
Copy link
Member

WunderBart commented May 21, 2020

Steps to reproduce the issue

  1. Add Image block,
  2. Click Select Image button and select Google Photos,

➡ The "Before/After Date" filter issue:

  1. At the top of the modal window, select the "Before/After Date" filter and add it,
  2. The filter element is added in a form of a link:
    Screenshot 2020-05-21 13 12 55
  3. Clicking the "Before/After Date" opens the calendar, but it always sets the current date regardless of what's picked.

➡ The "Only favorite media" filter issue (This should be fixed now):

  1. Add the "Only favorite media" filter,
  2. There's no space between the title and the "Remove filter" button:
    Screenshot 2020-05-21 13 55 23
  3. The favorite media filter has different wording for the select option vs added filter (might be resolved by External Media: Implementation #15717 (comment)):
    • Select option: "Favourite media only" (British),
    • Added filter: "Only favorites" (US).

➡ The "Category" filter issue:

  1. Add "After date", "Before date", "Category" and "Favourite media only" filters in this exact order,
  2. Remove "Only favorites" and "Category" filters in this exact order,
  3. Add "Category" filter again,
  4. Only the "Remove filter" button is added this time and the filter is still available from the select options:
    Screenshot 2020-05-21 14 08 31

Demo screencast:

Screen Capture on 2020-05-27 at 18-26-24

@WunderBart WunderBart added [Type] Bug When a feature is broken and / or not performing as intended [Extension] External Media Extend all block editor media tools to support external providers labels May 21, 2020
@WunderBart WunderBart changed the title External Media: Fix Google Photos filters UI External Media: Fix Google Photos filters May 21, 2020
@marekhrabe
Copy link
Contributor

To me the whole filter looks like a proof of concept, without much design applied.

Depending on how much work will be on it, I wonder if we could launch without configurable filtering at first (applying some simple "latest photos" default filter) and work on it as an update.

@getdave
Copy link
Contributor

getdave commented May 27, 2020

Depending on how much work will be on it, I wonder if we could launch without configurable filtering at first (applying some simple "latest photos" default filter) and work on it as an update.

I worked on something similar during my trial. I'd like to flag how much of a UX problem it is if you cannot filter your Google Photos. Rarely do I just want my most recent photos. We shouldn't underestimate this.

That said, we could ship MVP without filters but as long as we had a roadmap to bring them back asap.

@getdave
Copy link
Contributor

getdave commented May 27, 2020

Here's my Calypso PR where I had a "design" for date filtering

Automattic/wp-calypso#25762 (comment)

@davemart-in
Copy link
Contributor

Thanks for adding the demo screencast @WunderBart!

I'd like to flag how much of a UX problem it is if you cannot filter your Google Photos. Rarely do I just want my most recent photos. We shouldn't underestimate this.

Thanks for weighing in @getdave. I've got about 15 years worth of my photos in Google Images, so I know what you mean.

I think my proposal would be to simply drop the UI that is in there now and add an optional year and month filter. It's obviously not perfect, but it feels like a happy medium where users can at least A) Utilize filters within the Google Images UI to locate the right year/month, then B) use the year/month filter to locate the image within the our UI.

@jancavan jancavan self-assigned this May 28, 2020
@getdave
Copy link
Contributor

getdave commented Jun 1, 2020

Here is another idea I had for date filters a long time ago p4hfux-4s0-p2#comment-6378.

@obenland obenland changed the title External Media: Fix Google Photos filters (5P) External Media: Fix Google Photos filters Jun 1, 2020
@getdave
Copy link
Contributor

getdave commented Jun 1, 2020

@Automattic/ajax agreed we'd look into fixing the Date filter issues first and then if that becomes time consuming we'll revert to @davemart-in's idea of simplifying the UI.

@jancavan also suggested taking a look at revising the UX for filters entirely.

@marekhrabe
Copy link
Contributor

marekhrabe commented Jun 2, 2020

The calendar UI dependency Gutenberg is using allows for a much better UX for selecting a date range. The unfortunate part is that it does only expose the single day picker at this moment.

It might be possibly to also make the date range selection available but we'll have to wait a few WP releases until it's safe to use with Jetpack

These are all the features of the dependency: http://airbnb.io/react-dates/

@marekhrabe
Copy link
Contributor

I have started looking into the broken calendars now. They work differently with Gutenberg plugin active (eg: not broken) so I'm investigating what's up

@jancavan
Copy link

@jancavan also suggested taking a look at revising the UX for filters entirely.

That's being tracked here: #16153

@jancavan
Copy link

jancavan commented Jun 16, 2020

I think my proposal would be to simply drop the UI that is in there now and add an optional year and month filter.

Basic v1

Is a prototype of Dave's proposal above, a basic version with just a month/year filter.

Clickable prototype

basic-001c

  • Apply and Clear buttons are disabled until user inputs a month/year
  • User can clear form data using the Clear button

Mobile view:

Screen Shot 2020-06-16 at 9 54 20 AM

@Automattic/ajax agreed we'd look into fixing the Date filter issues first and then if that becomes time consuming we'll revert to @davemart-in's idea of simplifying the UI.

Still working on mockups for a less basic version (Basic v2) than this - ie: retaining the current filtering options, but a more finessed version.

@jancavan
Copy link

@Automattic/ajax agreed we'd look into fixing the Date filter issues first and then if that becomes time consuming we'll revert to @davemart-in's idea of simplifying the UI.

Basic v2

This is a visual improvement of the current version in production. The functionality and filter options remain exactly the same, except:

  • I'm making use of @wordpress/componentsfor the form elements
  • For the redundant Remove filter buttons, I replaced them with x icons

Clickable prototype

basicv2-001

For the calendar:

  • Let's remove the time field as its not necessary

Screen Shot 2020-06-16 at 2 11 45 PM

  • Mirror publishing date format - dd / month / yyyy vs month / dd / yyyy

Mobile view:

Screen Shot 2020-06-16 at 2 20 06 PM

Screen Shot 2020-06-16 at 2 19 31 PM

@marekhrabe
Copy link
Contributor

I'm all for the simpler solution now. I've spent (probably way too much 😢) time trying to figure out issues with the Calendar, described in the first post of this issue, and more I looked, more issues I've uncovered that block us from using the full calendar UI. Implementing Basic v1 should be no problem technically.

However, since we already have all the logic in place for those other filters, adding/removing them etc… would is make sense to go somewhere between Basic v1 (only date) and Basic v2 (similar to the current solution)? The difference from Basic V2 will be that instead two filters "before date" & "after date", we will make a new one called maybe just "Date" and it's functionality would look exactly what Basic v1 proposes (simplified date selection).

Would something like that make sense?

I've done a deep dive on what is causing the trouble with the calendar in dropdown. Click to read if you are brave

I've spend too much time trying to figure out the situation around the date filter in Google Photos modal of the external media.

In general, we are dealing with focus/blur fights in our own components and those from Gutenberg (also they behave slightly differently based on the Gutenberg version). The calendar as-is works but we run into so many unfortunate things together it's tough to untangle and fully understand. Here are some of my observations:

  • when you add the before/after filter, it gets initialized with an empty string value and you can now click the button to open the calendar selector to select some date
  • the date selector is using Dropdown from Gutenberg, which internally relies on another component Popover
    • this might or might not be important because Popovers are using a react portal and they get rendered elsewhere in the DOM, not within our media modal window, so many of our focus/blur expectations might be broken
    • Popovers in Modals are known to be buggy and none of alternatives mentioned helped Dropdown in Modal Doesn't Work WordPress/gutenberg#19453 (I've mainly spent time on SlotFillProvider and Popover.Slot placement within the Modal)
  • when the dropdown with date opens, the first element in it is focused - it's the month <select> element
  • now, when you try clicking anything but the month selector, the calendar component will first handle blur event on the <select> and it will call onChange with the current date (because that's what the date selector shows by default and by the <select> blur, you've "confirmed" the selection in its eyes)
  • the logic that implements onChange in our Google Filters immediately closes the dropdown when it gets a date from onChange. This way, your click on any other element won't even be registered as the blur handler has closed the whole UI on you
  • if that's not enough, our onChange also has to save the value of the selected date. So it does that and that means our logic to filter photos kicks in, shows image loading placeholders and calls the API
    • once the API succeeds, we automatically switch focus to the loaded content so even if we made the dropdown stay opened, it's focus will be lost and Dropdown closed
  • on top of that, the calendar component itself in Gutenberg (which uses an external library), for almost a year includes some temporary code that handles focus changes too Fix: changing month or day in post publish date closes the popover. WordPress/gutenberg#17201
    • and I didn't dig deeper into the actual calendar component, which comes from a 3rd party dependency and likely does other focus/blur handling

Some other things I've tried:

  • initializing the filter immediately with a current date, to eliminate the effect of "blur" of the <select> but without much success
  • instead of the complex DateTimePicker, I've tried rendering only it's inner dependencies on their own DatePicker (the visual calendar) and <TimePicker> (confusing name, it includes all the month/day/year inputs, as well as time ones)

This is probably not the full list of things that are in play but it gives you the idea we have way too much things to watch and handle and each of the components is trying to handle focus well for their own isolated purposes but all the pieces together are falling as a house of cards. Franky, I have no idea how to get out of this mess. I've tried left and right.

Alternatives considered:

  • Removing date filtering abilites (pointed out by team mates who use Google Photos to be difficult to accept)
  • Using a simpler date picker - native html inputs (day, month, year). I haven't confirmed if they'll work in a Dropdown or not
    • this has a caveat that we need to implement a local state for the date value and only sync it with the filtering logic using
    • We also need to ensure proper i18n
  • third option that came to my mind recently is to look into react components already present in the Jetpack repo (if any are Calendars) - 50/50 chance they'll do strange things too, as calendar UIs tend to be complicated

I think the path of least resistance (except for removing date filters) is to make a very bare bones custom date selector, using basic HTML form inputs. We have planned to potentially rework the filter UI soon so I'd suggest to start as easy as we can and keep technical limitations observed in mind for the next iteration.

@obenland
Copy link
Member

Would something like that make sense?

Let's go with the path of least resistance, your suggestion makes sense.

@jancavan
Copy link

jancavan commented Jun 18, 2020

Thanks @marekhrabe for the thorough explanation. In addition to month/year filters, I'd like to suggest that we provide preset time periods as well. Here's a prototype to illustrate what I mean:

Better v1

Custom-date-range-001

Let me know if this is feasible. Since calendars are off the table for now, we can replace it with month/year filter instead. I'll have to mock that up tomorrow, as well as a version that doesn't use dropdowns.

@davemart-in
Copy link
Contributor

Thanks for all of the demos @jancavan!

@marekhrabe let's go with the initial preset dropdown Jan mocked up here. It sounds like the calendar component is giving us some grief. For the "Custom date range" option, if we can make the start/end date calendars work, great. I'm assuming we can't based on your comment above. No worries. If not, I'm 100% happy to ship a v1 with the simple month and year fields that Jan prototyped here.

Let me know if you have any questions.

@marekhrabe
Copy link
Contributor

marekhrabe commented Jun 18, 2020

Sounds good and the preset ranges are easily doable too. One thing I want to confirm whether we should remove all other filter options (favs, categories…) and only include the date or if we should keep filters and only update the method for entering dates.

@davemart-in
Copy link
Contributor

Yes, lets remove all other filter options for this v1.

Note: I'm happy to revisit this and add them back in if enough customers specifically ask for it.

@marekhrabe
Copy link
Contributor

Sounds good!

@marekhrabe
Copy link
Contributor

One more check - what about the album/recent select? That one works without a problem - keep it?

@obenland
Copy link
Member

I'd be in favor of keeping it, albums seem to be the most prominent way of organizing images in Google Photos. We've also already spent time to make it work correctly.

@davemart-in
Copy link
Contributor

Yep, if they work. Let's leave those in there. Thanks!

@marekhrabe
Copy link
Contributor

First phase ready for a review: #16190

Included:

  • additional filters hidden
  • only date filter present - showing predefined date ranges

What will follow:

  • ability to set a custom range
  • matching the visual design (right now it's purely functionality-based)

@jancavan
Copy link

jancavan commented Jun 19, 2020

Since calendars are off the table for now, we can replace it with month/year filter instead.

Better v1.1

As promised, here's another iteration of "Better v1", but with the month/year custom filter added:

better-v1 1

@davemart-in
Copy link
Contributor

@jancavan a couple of thoughts here:

  • I'm not sure that the "Sort by time period" dropdown should dynamically change to include an option for "June 2020" once a custom date is selected. The "Custom Month/year" should stay selected and visible at all time.
  • Rather than use a popover box for the month and year selection, can we just align these in the same row as the "Sort by time period" dropdown, like this:

options

@jancavan
Copy link

jancavan commented Jun 21, 2020

Thanks for your feedback, Dave!

I think my concern with that approach is it clutters up the UI a bit.

It also looks like "View" is affected by the "Apply" button. We could fix that by visually separating the two groups, but now the entire filtering functionality is separated from what it's supposedly filtering:

Screen Shot 2020-06-20 at 8 59 49 PM

Thinking about this more though, I think we should add further affordance that there are more options when the user selects "Custom" like so (we have a similar style for the Navigation Block):

Screen Shot 2020-06-20 at 6 06 06 PM

Here I separated it with a divider from the rest of the options and added a ...

With these additions, I think a dialog box approach is probably more fitting. We could contract it a bit more though so it doesn't take up a lot of horizontal space:

Screen Shot 2020-06-20 at 9 18 12 PM

I'm not sure that the "Sort by time period" dropdown should dynamically change to include an option for "June 2020" once a custom date is selected. The "Custom Month/year" should stay selected and visible at all time.

Hmm, not sure if you're only referring to the mockup above, or if you meant this in general. If the latter, I think displaying the selected values ("June 2020" in this example) gives it more context; the "Custom month/year" label is only an indicator that this value can be customized.

@davemart-in
Copy link
Contributor

davemart-in commented Jun 21, 2020

@jancavan can you help me visualize what the user sees after this screen?

image

Changing the existing dropdown values doesn't make sense to me. I don't think I've ever seen a dropdown do this. It feels unconventional.

@jancavan
Copy link

jancavan commented Jun 21, 2020

Hey @davemart-in, here's a quick prototype:

custom-date-range-filter

If the user clicks on the dropdown again though, this option will remain "Custom month/year...", it won't change to "June 2020".

This is just to set context; so it's clear to the user what they set "Custom month/year" to.

This is an example I see in Gmail:

Screen Shot 2020-06-21 at 1 58 55 PM

Screen Shot 2020-06-21 at 1 59 11 PM

Box:

Screen Shot 2020-06-21 at 2 06 40 PM

Screen Shot 2020-06-21 at 2 07 49 PM

  • I think the only difference with ours is we only have month/year instead of an actual date range
  • Select box should also indicate active state

Another option is to keep them all in the same control, but we'd have to customize the dropdown:

Screen Shot 2020-06-21 at 2 33 08 PM

@davemart-in
Copy link
Contributor

@jancavan thanks for sharing those examples. I had never seen this pattern before.

This sounds complicated on the implementation side of things, but I'll let @marekhrabe weigh in there. It seems like we'd have to dynamically update the options within the select menu once a month/year has been selected, and then dynamically remove that option if you click into the dropdown menu again.

@marekhrabe
Copy link
Contributor

I'll dig into this a bit more later. I can only confirm that whatever label is visible when the select is closed must be a valid option there and will be in the select when it gets opened. There is now way around it, unless we custom code everything and skip using Gutenberg components.

I'm thinking a good solution might be to add the selected range as a proper item in into the list once user applies it. Options could look like this after applying:

  • Any time
  • Last 12 months

  • June 2020 (selected)
  • Custom month/year…

Selecting "Custom" will again open up the possibility to enter custom values. Those will always override the "June 2020" item in the list. Would that work for y'all?

@jancavan
Copy link

Thanks @marekhrabe. To clarify, in the prototype, when the user has entered custom values ("June 2020" in this example), the exposed value in the dropdown will change to "June 2020", but once they expand the dropdown again, it will remain "Custom month/year".

Similar to Gmail:

Collapsed:
Screen Shot 2020-06-22 at 8 28 20 AM

Expanded:
Screen Shot 2020-06-22 at 8 24 06 AM

But what you're saying is, the way we can go about it is to append the custom value in the dropdown list like so:

  • ...
  • June 2020 (selected)
  • Custom month/year…

If they enter additional custom values, I'm assuming that will replace "June 2020", and not keep appending new custom values to the list?

@marekhrabe
Copy link
Contributor

but once they expand the dropdown again, it will remain "Custom month/year".

I got the intention but it cannot be replicated with the Select component from Gutenberg. Whatever is in the closed dropdown must also be in the expanded one and must be selected. Gmail is using completely custom code for the dropdown so they can make it more "magical" but Gutenberg exposes just the most basic implementation <select>.

If they enter additional custom values, I'm assuming that will replace "June 2020", and not keep appending new custom values to the list?

Up to you. My thinking was to only keep the lastest selected in the list and overwrite each time. Append can however also be implemented, if you want to go that way.

@jancavan
Copy link

Up to you. My thinking was to only keep the lastest selected in the list and overwrite each time. Append can however also be implemented, if you want to go that way.

Yes, I think it makes sense to overwrite. What do you think @lessbloat?

@davemart-in
Copy link
Contributor

davemart-in commented Jun 22, 2020

We'd still need to code up a custom popover component just to make this work (for the month and year dropdown). I'm not sure the technical complexity that this adds leads to that much better of an experience.

My preference would be to just stick with something like:

image

which feels straightforward to implement and easy for customers to understand at a glance. The width of the sort by and Month select fields could likely be reduced once implemented to make for a more compact filter UI.

@jancavan jancavan self-assigned this Jun 22, 2020
@jancavan
Copy link

Here's a prototype @marekhrabe of a more simple, and straightforward approach as per @lessbloat:

progressive-discolsure-002

  • "Apply" button should be disabled if user has not added any custom month/year values:

Screen Shot 2020-06-22 at 11 35 41 AM

  • User can fill out either month or year and this enables the "Apply" button. This way, they can filter by year only, but has to input at least 4 numerical values for year to enable the button:

Screen Shot 2020-06-22 at 11 38 12 AM

  • Let's not forget to change the form elements to its focused state as the user is interacting with them.

Mobile view:
Screen Shot 2020-06-22 at 11 43 03 AM

@obenland obenland changed the title (5P) External Media: Fix Google Photos filters (5P) External Media: Remove existing Google Photos filters in favor of date-range dropdown Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Extension] External Media Extend all block editor media tools to support external providers [Status] Needs Design [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants