Fix #1260: Added frontend support for autocomplete #1411
Conversation
There was a problem hiding this comment.
This is really good stuff. I made some comments on things to fix, then I think this is pretty close to being ready.
One other thing we need is for you to file a new issue to add the backend changes for doing the author name autocomplete via our REST API, and hooking it up to this frontend code. You can link back to this PR so people can see what you did here, and what's left.
There was a problem hiding this comment.
This is excellent, great job.
Based on my testing, I want two more small CSS tweaks.
Also, do we need to repeat the input style in both Author and Post search components? Could we not move that up to the parent SearchInput component and only do it once? Duplicate code like this is problematic because people will change one but forget the other. If they have to be split like this, that's fine, but it would be good to DRY this code out a bit more.
Finally, you need to rebase to fix a merge conflict with master. To do that:
git checkout master
git pull upstream master
git checkout issue-1260
git rebase master
...fix the conflict
git add <file-you-fixed>
git commit -m "..."
git push origin issue-1260 -f
I'd love to get this merged today if possible, so I can include it in the 1.4 release.
| height: '55px', | ||
| backgroundColor: theme.palette.background.default, | ||
| paddingLeft: '10px', | ||
| border: '1px solid #B3B6B7', |
There was a problem hiding this comment.
Let's add an outline: none, here too so we don't get a double border when selected.
| height: '55px', | ||
| backgroundColor: theme.palette.background.default, | ||
| paddingLeft: '10px', | ||
| border: '1px solid #B3B6B7', |
There was a problem hiding this comment.
Let's add an outline: none, here too so we don't get a double border when selected.
MSTEWARDSON
force-pushed
the
issue-#1260
branch
from
9a9cebc to
7224af8
Compare
| @@ -0,0 +1,60 @@ | |||
| import React from 'react'; | |||
| <<<<<<< HEAD | |||
There was a problem hiding this comment.
Your rebase didn't work, can you fix this?
| @@ -0,0 +1,126 @@ | |||
| import React from 'react'; | |||
| import PropTypes from 'prop-types'; | |||
| <<<<<<< HEAD | |||
|
When I get rebase now it says the current branch is up to date already. |
|
It's up to date in terms of commits being in your line of development, but you haven't successfully dealt with merge conflict. All the conflict markers are still in those files. Look at the code. |
|
Yeah I'm currently looking. Fixed the issue within AuthorSearchInput.jsx, but I do not see the issue within SearchInput.jsx |
|
Filed #1414 to add the backend support we need to make this feature work. cc @StellaJung-Student, @joelazwar as well, who have been working on the Elasticsearch backend too. |
|
Closing in favour of #1418 |
Issue This PR Addresses
Fixed #1260
Type of Change
Description
This PR is for implementing the front-end portion of the author autocomplete. With this SearchBar can now pick between two different input's depending on the filter value. From here the author input has a data-list attached to it that I can later add in the back-end support for all author names.
Currently there is some fake data in the data-list just to show how it works. If approved I will remove before merge.
Checklist