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

feat(BB-432): Show possible duplicates next to the name section #814

Merged
merged 6 commits into from
Sep 1, 2022

Conversation

the-good-boy
Copy link
Contributor

@the-good-boy the-good-boy commented Mar 16, 2022

This PR addresses ticket BB-432. The suggestions no longer appear just below the name field.
This is how it looks like when there are more than 5 suggestions:
step1
The list of results is scrollable, so its height does not go over what is show in the screenshot above

This is how it looks like when there is an exact match:
step3

Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far so good :)
It is a bit disturbing to see the inputs on the left like that when there is nothing to the right, but perhaps it is only because I am used to the current placement.

Two things are sure:

  1. It's better than the current solution
  2. it's better to have the inputs on the left all the time than to change the layout automatically (for example having input in the middle, but move them to the left if we need to show the duplicates to the right).

Out of curiosity, did you try with a popup sort of functionality?
I made a quick and dirty mockup based on the tooltips we have on the input labels [the (?) icons], I think it's a solution that could work, although I'm not sure how we would handle the placement.
Worse comes to worse we can always roll out some custom CSS, which I can help with.

image

src/client/entity-editor/name-section/name-section.js Outdated Show resolved Hide resolved
src/client/entity-editor/name-section/name-section.js Outdated Show resolved Hide resolved
src/client/entity-editor/name-section/name-section.js Outdated Show resolved Hide resolved
@the-good-boy
Copy link
Contributor Author

About the popup functionality, can you tell me a little more. Do we want there to be a button/icon which gives us an alert that there are possible duplicates and clicking that button displays that popup on the side? Or is it something which automatically pops up whenever we have suggestions?

@the-good-boy
Copy link
Contributor Author

I just realized that there are some more mistakes here. When we view on smaller screens, the suggestions columns bleeds outside the entity-editor like this:
suggestions

Putting a margin-left-1 on the outer div kind of fixes that, but then the entire name-section gets a margin left, and stands out if one looks closely
Screenshot from 2022-03-24 19-39-36

@MonkeyDo
Copy link
Member

About the popup functionality, can you tell me a little more. Do we want there to be a button/icon which gives us an alert that there are possible duplicates and clicking that button displays that popup on the side? Or is it something which automatically pops up whenever we have suggestions?

It turns out I don't have a great solution for this, so let's go with what you currently have :)

Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last little bit and this will be ready to merge, thanks :)

Comment on lines 226 to 227
<Col md={{span: 6}}>
<Row>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took me a little while to find the issue you mentioned with the weird margins on smaller screens:

The way these are used is that Row elements contain Col elements instead of the other way around.

The .row class has negative left and right margins which explains what we are seeing when collapsed;

I think the simple solution here is to replace these <Row> elements here with <div>s

@the-good-boy
Copy link
Contributor Author

Hey @MonkeyDo do you think that perhaps we can further improve this by removing the "Show More" button and instead using a fixed-size scrollable list (something like this). That way we don't get all that whitespace on the left when we expand the list. I haven't tried this yet, its just something I was considering.

@MonkeyDo
Copy link
Member

Hey @MonkeyDo do you think that perhaps we can further improve this by removing the "Show More" button and instead using a fixed-size scrollable list (something like this). That way we don't get all that whitespace on the left when we expand the list. I haven't tried this yet, its just something I was considering.

That's a great idea, I love it.
It will certainly improve the view.

P.S: I took the liberty of fixing a merge conflict, since I knew which bit had changed. Sorry it's taking so much time for me to get to these PRs that they accumulate merge conflicts 😓

Instead of a "show more" button and a very long list of results, let's make the list of search results scrollable, showing up to 5 results at a time
@MonkeyDo MonkeyDo changed the title [BB-432]:Dont change shape of name-section due to searchResults feat(BB-432): Show possible duplicates next to the name section Sep 1, 2022
@MonkeyDo MonkeyDo merged commit a61643a into metabrainz:master Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants