-
Notifications
You must be signed in to change notification settings - Fork 16
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
Display search results as cards #2055
base: develop
Are you sure you want to change the base?
Conversation
<span class="size-small" [matTooltip]="entry.topic" matTooltipPosition="left"> | ||
{{ entry.topic }} | ||
</span> | ||
<app-ai-bubble *ngIf="entry.topicSelection === TopicSelectionEnum.AI && !entry.isApprovedAITopic"></app-ai-bubble> |
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 updated the cards in the collection so they match the search cards more, i.e. the AI bubble is after the topic instead of before.
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 the new component that is used for all entry type search tables. It contains alot of the code from the old individual entry type tables, in addition to the tag cloud code, which is moved from search-results.component
to this component
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.
These styles were moved from search-results.component.scss
to here
@@ -40,21 +40,18 @@ | |||
<img class="site-icons-tab m-2" src="../assets/svg/sub-nav/workflow.svg" alt="workflow icon" /> | |||
<b>Workflows</b> | |||
</ng-template> | |||
<div class="mat-tab-content"></div> |
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 class didn't actually exist in the component scss file, so removed
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, Kathy! Before reviewing the code, some questions/observations about the design/layout/UI itself:
- If no author exists, omit the "authors" line/info completely?
- Circular "authors" icon is similar to circular "entry type" icon, and thus dilutes the utility of the "entry type" icon as a visual "bullet". Change to something different, perhaps an icon similar to the mockup, or change the icon to text ("Authors:")?
- The mockup has less spacing between successive lines of the topic text. Saves a bit of vertical space and, IMHO, looks nice. Replicate in the actual version?
- Is sorting by "Least Stars" gonna be useful to anyone?
- Some other potentially-useful sorting criteria: Recently Updated
- To save vertical space, put authors somewhere else? Could float it somewhere, maybe in a block with an author per line.
- The "Popular Keywords" button is still visually obnoxious, should we further deemphasize?
- Horizontal spacing between "last updated" text and bubbles is tight. Increase?
- "Star number" appears slightly higher on the page than the accompanying star. Fix?
Would love to hear what everyone thinks!
Forgot to note that none of these are "mandatory" changes, but rather they're some ideas/etc that we might consider/implement (or not). |
Very satisfying, looks really good! I agree with most of @svonworl's comments (both in terms of nice to have but not essential and/or could be follow-up tickets , just noting the ones with a bit of disagreement)
I can see where this is coming from but I also like the "advertising" or get your name here aspect of things. It fits well with the drive toward more DOIs as well. Is there a way to save space but also act as a subtle hint that users should tie in some form of authorship whether ORCID or whatnot?
Kinda like this as a fun touch, but the grey is fine. |
Description
Webservice PR: dockstore/dockstore#6071
This PR turns the search results into search cards based on the mockups shared in dockstore-devs. Alot of the code between the various entry type search tables were the same, so I refactored it into one component.
Slightly different from the mockups: I moved the "Popular Keywords button" down so that it's on the same line as the "Sort by" selector because it looks better IMO and it prevents the tag cloud from blocking the Sort by options which would've happened in the original mockup. Note that the Popular Keywords button background color is slightly less vibrant because I modified it to use the existing
<entry-trype>-background-color
classes.For the "Sort By" options, you can only sort by Stars, Name, and Authors. I removed the descriptor type because the sort by drop down was getting long with all the options and descriptor type sorting didn't seem as helpful since we have descriptor type facets.
Original mockups for reference
Without highlighting
With highlighting
Before
Without highlighting:
With highlighting:
After
Search cards:
Sort by options:
With DOIs:
Tools:
Notebooks:
Search highlighting:
Review Instructions
Go to the search page and interact with it by selecting facets (languages, categories, etc) and verifying that the search result cards get updated accordingly. Verify that pagination works as expected. Interact with the Popular Keywords tag cloud and verify it works. Repeat for the other entry search tables.
Issue
dockstore/dockstore#5726
https://ucsc-cgl.atlassian.net/browse/DOCK-2482
Security
If there are any concerns that require extra attention from the security team, highlight them here.
Please make sure that you've checked the following before submitting your pull request. Thanks!
npm run build
markdown-wrapper
component, which does extra sanitizationnpm audit
and ensure you are not introducing new vulnerabilities