Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Post author dropdown: add accessible-autocomplete #7385
Post author dropdown: add accessible-autocomplete #7385
Changes from all commits
ea01c26
f84c371
240c304
0d1991d
ea094fb
642ae6f
4ab2777
3ebc2c2
a526c60
a8e9bb1
aad6dcb
16dd20c
b056747
6e69cf0
12e58b0
14edb8a
6d555c7
853dfe6
83a7be4
940f338
18290b2
42c6a60
1e322aa
8da09fc
e31b540
7a70911
7e7696b
6c765c7
1b10c63
7aab164
4e47a3f
03e0155
3e33275
6238e4c
9d1b490
efac333
7ec4b6c
8a9e711
5173a7a
e4915b0
86b3d31
d6e1deb
6bdba75
a683a1c
46d27f9
c3bf43d
877032a
9e42571
43ccd6c
79b6c66
708fd55
aa5376f
4f94442
1ecfa05
c6a9b18
06b3cdd
319c96c
a97e293
839a618
550c63a
d1731ec
6585f70
ca3e4be
66daf9f
74e4083
c70e125
cdf7bde
4aded2f
f1b5739
d227dfb
205f07a
790c2c2
cb94e71
729037d
849949c
91a891d
2f1b5e2
76090f7
e6a8713
74e4f71
ac7bedd
783655c
180cd54
e45ffe0
525cf40
93b5396
9a35172
31d50b5
50400fc
ac21e7b
800787e
a021841
3fdd917
741a2a8
9862909
96ed175
1980080
624c49d
fcaf60d
da3de52
d3443e7
1217076
1d13ad8
8d3ae5b
db3bea3
807b61e
5ba93d1
0c7d6b5
03fc448
aa4bbcc
b3839b3
e593595
d528303
27cedb3
7e5d82d
4c9fd8b
4d8c249
b830022
2426493
bd6ef21
0b12bfd
28bf036
2daaa8c
1e824ad
c89d301
8d7db70
43d4a59
4c6a447
83f6086
3317851
615e3d9
cbea8ff
0066508
ba26d03
eb64f3b
24ddea3
92e4074
71b7243
73343ae
e445aa7
6e129da
e545898
1c1fb89
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally a component should never be calling
apiFetch
directly, and rather use the core data resources as the canonical source for REST API entities. One issue with how this is implemented is we have nothing to cancel the behaviors from taking place if the component suddenly becomes unmounted, potentially leading to errors.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll switch this to using core/data adding whatever support we need there.
will core/data account for that or should i be endeavouring to cancel these requests when the component unmounts? another option would be tracking mount state and doing ending silently for request when unmounted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a non-concern for you. In practice, it doesn't actually cancel it, but it doesn't really matter, and also helps if a future component mount also needs the same data (another reason why instance-specific fetches should be discouraged, since the data can / should be shared).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we also use
apiFetch
in the user autocompleter - https://github.com/WordPress/gutenberg/blob/master/packages/editor/src/components/autocompleters/user.js#L20 - i think to replace thse we'll need a new resolver, perhapssearchAuthors
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aduth Thinking about adding the capability to select both a single user by ID and to search users via the data component - currently this does not seem possible.
Do you think I should add this to the existing
getAuthors
selector, eg the ability to pass an id or search query to the selector? or create something new?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aduth any feedback on this question?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's some precedent here with the existing "entities", currently post types, taxonomies, and media items. The entities implementation will auto-generate two selectors:
Considering
getAuthors
as a sort of partially-applied entities selector, I could see this being applied as either:getAuthors
and creating a newgetAuthor
selectorgetUsers
And, in fact, it could be both, where the first is just an abstraction on the second.
I'm not sure if I've missed anything, but I'd rather not have
getAuthors
diverge as being its own special thing if it's possible for us to bring it in line with these other standard entities behaviors.