Fetch all authors from REST API and make dropdown scrollable

[ndev1991]

Issue #756

Add scrollbar inside dropdown and set the max-height.

[github-actions]

Size Change: +330 B (0%)

Total Size: 511 kB

Filename	Size	Change
assets/js/edit-story.js	440 kB	+330 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
assets/css/edit-story.css	3.01 kB	0 B
assets/css/stories-dashboard.css	206 B	0 B
assets/js/stories-dashboard.js	68.2 kB	0 B

_{compressed-size-action}

[barklund]

I think the larger issue here is, that only 10 users are loaded from the db. All db users should be loaded and selectable in the dropdown.

CC @spacedmonkey

[spacedmonkey]

I think the issue here is that, the REST API only returns 10 by default. The api maxes out at 100. As to get all users, we would have to pagination through all users.

[spacedmonkey]

Now we have loading from multiple api request, it take a look time for the api requests to load. Should we check if users is not empty on this line now?

[spacedmonkey]

Shouldn't this dropdown be wider?
I am looking at this in chrome and firefox on linux.

[ndev1991]

Now we have loading from multiple api request, it take a look time for the api requests to load. Should we check if users is not empty on this line now?

I think users is null at the beginning and updated when there is values. And also we have checking users like && users &&.

[spacedmonkey]

There is already a check, but I am seeing an empty dropdown for a bit.

[ndev1991]

There is already a check, but I am seeing an empty dropdown for a bit.

Okay. Then we may need that to dropdown.

[spacedmonkey]

Should be check if see if not not null users and length is more than 0?

[ndev1991]

Should be check if see if not not null users and length is more than 0?

Yes, basically we have checking point for users not null for that part. And we will need to add length check to dropdown I think.

[ndev1991]

Should be check if see if not not null users and length is more than 0?

Yes, basically we have checking point for users not null for that part. And we will need to add length check to dropdown I think.

hmm... that maybe doesn't make sense. It would be good to add length check after && users &&

[ndev1991]

@spacedmonkey basically if we add that beside users check in then the author line with dropdown will appear after all the users loaded. And if we add that to dropdown then the author line shows and the correct value set for dropdown after users fully loaded. What do you think about this? Which would be the expected behavior for this?

[spacedmonkey]

@spacedmonkey basically if we add that beside users check in then the author line with dropdown will appear after all the users loaded. And if we add that to dropdown then the author line shows and the correct value set for dropdown after users fully loaded. What do you think about this? Which would be the expected behavior for this?

The user dropdown loads after in gutenberg. The alternative here to a dropdown that doesn't work at all. The best UX would be only load the users when the dropdown is clicked, as they may never use the dropdown at all. But that is more work.

CC @pbakaus @swissspidy What do you think? Hide the author dropdown until all users have loaded or user something and have a possibly none functioning dropdown for a while.

[pbakaus]

@spacedmonkey basically if we add that beside users check in then the author line with dropdown will appear after all the users loaded. And if we add that to dropdown then the author line shows and the correct value set for dropdown after users fully loaded. What do you think about this? Which would be the expected behavior for this?

The user dropdown loads after in gutenberg. The alternative here to a dropdown that doesn't work at all. The best UX would be only load the users when the dropdown is clicked, as they may never use the dropdown at all. But that is more work.

CC @pbakaus @swissspidy What do you think? Hide the author dropdown until all users have loaded or user something and have a possibly none functioning dropdown for a while.

I would definitely recommend loading it async (not on demand when clicking the dropdown, but also not in the critical path of the editor).

So:

1. on load, show a placeholder for the dropdown ("Loading..", reserving the same space so it doesn't jump around of course)
2. After the editor is fully initialized, start loading the authors
3. Replace placeholder with dropdown.

Relatedly, bonus points for loading the entire JS for this async, and not have a single bundle for everything.

[spacedmonkey]

@ndev1991 Did you try passing -1 to the api?

See this comment. WordPress/gutenberg#21212 (comment)

[ndev1991]

@ndev1991 Did you try passing -1 to the api?

See this comment. WordPress/gutenberg#21212 (comment)

@spacedmonkey yes. It's same with 100. Returns 100 users at a time when using -1

[spacedmonkey]

LGTM

[swissspidy]

Just to clarify: passing -1 didn't work, despite the fetchAllMiddleware effectively doing the same thing as this PR? 🤔

[ndev1991]

Just to clarify: passing -1 didn't work, despite the fetchAllMiddleware effectively doing the same thing as this PR? 🤔

@swissspidy Yes, passing -1 didn't work. It's same as per_page=100. But not tested fetchAllMiddleware

[swissspidy]

@ndev1991 Please test this again. The following code automatically resulted in multiple HTTP requests being made, loading all users on the site:

const getAllUsers = useCallback(() => {
    return apiFetch({ path: addQueryArgs(users, { per_page: '-1' })});
  }, [users]);

[swissspidy]

In my testing this just worked for me:

const getAllUsers = useCallback(() => {
  return apiFetch({ path: addQueryArgs(users, { per_page: '-1' })});
}, [users]);

Because api-fetch automatically handles this for us.

[ndev1991]

In my testing this just worked for me:

const getAllUsers = useCallback(() => {
  return apiFetch({ path: addQueryArgs(users, { per_page: '-1' })});
}, [users]);

Because api-fetch automatically handles this for us.

@swissspidy yes it's working. I didn't see it makes multiple http request to fetch all the users.
Updated the code and pushed! Thanks