Complete Marketplace Design #244
Conversation
dherault
added
enhancement
| @@ -71,6 +78,14 @@ function MarketplaceRepositories({ installed }) { | |||
| }) | |||
| .filter(repository => installed ? repository.installation : true) | |||
|
|
|||
| const fuse = new Fuse(sortedRepositories, searchOptions) | |||
|
|
|||
| const resultRepositories = search ? fuse.search(search).map(({ item }) => item) : sortedRepositories | |||
There was a problem hiding this comment.
Is this searching in-browser?
There was a problem hiding this comment.
Yes, this hack works well for now.
In the future we might want to use the open-source version of Algolia 🇫🇷 !
There was a problem hiding this comment.
no we should use the api's own search features.
There was a problem hiding this comment.
You should now have the categories, publishers and tags fields to do multi-value filtering for all of these via the api. We should definitely strongly prefer that to in-browser filtering
| @@ -21,7 +21,7 @@ export function ModalHeader({ text, setOpen }) { | |||
| justify="center" | |||
| width="2rem" | |||
| height="2rem" | |||
| hoverIndicator="background-light" | |||
| hoverIndicator="fill-one" | |||
There was a problem hiding this comment.
the name fill-one seems a little unclear to me, is there a way to make it a little more apparent what that color is acting as?
There was a problem hiding this comment.
Ask Nick, that's his responsibility
There was a problem hiding this comment.
We can bring it up with nick too, but for codebase cleanliness, I do think we should have a mapping of semantic names that correspond to his more systematic ones.
| width={areFiltersOpen ? 256 - 32 : 0} | ||
| height={`calc(100% - ${32}px)`} | ||
| right={0} | ||
| width={256 - 32} |
There was a problem hiding this comment.
is there a reason why we don't just make thse constants?
| height={`calc(100% - ${32}px)`} | ||
| right={0} | ||
| width={256 - 32} | ||
| height={`calc(100% - ${64 + 8}px)`} |
There was a problem hiding this comment.
same shouldn't this just be a const?
| function renderTags() { | ||
| const sortedTags = tags.slice().sort((a, b) => a.name.localeCompare(b.name)) | ||
| const fuse = new Fuse(sortedTags, searchOptions) | ||
| const resultTags = search ? fuse.search(search).map(({ item }) => item) : sortedTags.filter((x, i) => i < nDisplayedTags) |
There was a problem hiding this comment.
we're definitely not going to be able to load all tags in one fetch in relatively short order, so this should really be moved to utilizing the gql apis properly
There was a problem hiding this comment.
I'm waiting on some green go from the backend team then.
There was a problem hiding this comment.
what do you need api wise? I'm pretty sure we do have the capability to filter tags
There was a problem hiding this comment.
yep, you can use the q argument for the tags query to filter by a search string
| @@ -61,16 +64,33 @@ function MarketplaceRepositories({ installed }) { | |||
|
|
|||
| const sortedRepositories = repositories.slice() | |||
| .sort((a, b) => a.name.localeCompare(b.name)) | |||
| .filter(repository => categories.length ? categories.includes(repository.category) : true) | |||
| .filter(repository => categories.length ? categories.includes(repository.category.toLowerCase()) : true) | |||
There was a problem hiding this comment.
sorting should also be delegated to the api
There was a problem hiding this comment.
I've added a ticket for you to do it.
|
Scrollbar should appear on right edge of browser window. Otherwise, looks great. |
|
Will work on it. |
|
Marketplace scrollbar is good. I should have caught this earlier, but Filters sidebar is overflowing horizontally and showing a horizontal scrollbar.
|
|
I'm still seeing the bottom margin for the filters box being far too big, it should be closer to the bottom of the screen |
Summary
Resolves ENG-118
Finalizes the layout for the "Marketplace" and "Installed" scenes.
## LabelsTest Plan
Checklist