-
Notifications
You must be signed in to change notification settings - Fork 22
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
Feature/4628 search for articles to add #1990
Feature/4628 search for articles to add #1990
Conversation
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.
This is great progress @amoedoamorim
one thing that I didn't put into the designs (my oversight) is an "empty" message for when the search doesn't match anything.
I can put together designs if it's easier for you, let me know.
but my thought is to use the same styles that Caio added for the empty articles message. I believe the className is articlesSidebarNoArticle
however with the text:
No results matched your search.
Let me know if you have any questions.
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.
Code and functionality look good to me
And include unit test too
…ture/4628-search-for-articles-to-add
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.
thanks for adding the empty state @amoedoamorim !
Description
Introduces the
ChooseExistingArticleButton
References: CV2-4628
Type of change
How has this been tested?
Tested manually locally and added unit test for the open slideout functionality.
To test locally go to the new articles tab on an item and verify the new "Choose existing article button"
Things to pay attention to during code review
Please describe parts of the change that require extra attention during code review, for example:
Checklist
propTypes
are declared and they use React Hooks and, if data-fetching is required, they use Relay Modern with fragment containers