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

Suggest similar investigations based on the currently displayed investigation on investigation landing page #1522

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

kennethnym
Copy link
Member

@kennethnym kennethnym commented Apr 20, 2023

Description

This PR adds a section on investigation landing pages that suggests similar investigations based on the currently displayed investigations. Investigation suggestions and extra info for the investigation are grouped into 2 accordions for better visual clarity. This video demonstrates the new UI:

DataGateway.DataView.and.15.more.pages.-.Work.-.Microsoft.Edge.2023-04-27.15-15-15.mp4

The URL of the API that is used to query suggestions are hardcoded at the moment.

Testing instructions

Add a set up instructions describing how the reviewer should test the code

  • Review code
  • Check Actions build
  • Review changes to test coverage

Agile board tracking

Closes #1521

Add a section on investigation landing pages that suggests similar investigations based on the currently displayed investigations
@kennethnym kennethnym added enhancement New feature or request datagateway-dataview Issues corresponding to the dataview component. This component supersedes datagateway-table labels Apr 20, 2023
Update investigation landing page test for the new suggested investigations section
@agbeltran
Copy link
Member

This is great @kennethnym !!! Could we show a few more similar investigations, please? Also cc'ing @saifulkhan for him to comment.

@kennethnym
Copy link
Member Author

Absolutely, do you have a particular number in mind? Obviously we don't want to show too much items, otherwise it'll crowd the page too much. Is 5 a good number to you?

@codecov
Copy link

codecov bot commented Apr 20, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (0bdc641) 96.23% compared to head (9a6e15b) 96.15%.
Report is 5 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1522      +/-   ##
===========================================
- Coverage    96.23%   96.15%   -0.08%     
===========================================
  Files          161      162       +1     
  Lines         6935     7001      +66     
  Branches      2147     2169      +22     
===========================================
+ Hits          6674     6732      +58     
- Misses         240      247       +7     
- Partials        21       22       +1     
Flag Coverage Δ
common 95.48% <81.81%> (-0.06%) ⬇️
dataview 96.75% <89.65%> (-0.19%) ⬇️
download 96.32% <ø> (ø)
search 96.47% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
packages/datagateway-common/src/app.types.tsx 100.00% <ø> (ø)
...anding/isis/isisInvestigationLanding.component.tsx 100.00% <100.00%> (ø)
...ages/datagateway-common/src/api/investigations.tsx 98.94% <81.81%> (-1.06%) ⬇️
.../isis/suggestedInvestigationsSection.component.tsx 88.67% <88.67%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@agbeltran
Copy link
Member

maybe let's show 10 - or @saifulkhan can comment on how many would be better to show at this stage (notice that this is not the final UI, it is just for demo purposes)

Show more similar investigation suggestions on investigation landing pages
@kennethnym
Copy link
Member Author

Addressing feedback provided by @saifulkhan, I have added topic chips & pagination to the suggestion list. Demo below:

DataGateway.DataView.and.15.more.pages.-.Work.-.Microsoft.Edge.2023-04-27.15-15-15.mp4

I've updated the original comment as well with the new demo.

@kennethnym
Copy link
Member Author

kennethnym commented Apr 27, 2023

Placing the pagination control at the bottom causes it to be unstable, i.e. the control doesn't stay in the same position when switching between pages due to the varying height of the list, which means the users would have to re-aim the mouse cursor between page changes.

Make the suggestion list paginated and display relevant topics as chips
@kennethnym kennethnym marked this pull request as ready for review May 9, 2023 16:50
Copy link
Member

@louise-davies louise-davies left a comment

Choose a reason for hiding this comment

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

Code looks good to me, maybe just add a TODO comment on the hardcoded URL to remind us for later that it needs to be changed. Only commenting for now instead of approving because we're awaiting a demo of this feature which would unblock "doing it properly" e.g. not hardcoding the URL

@kaperoo
Copy link
Contributor

kaperoo commented Oct 4, 2023

Changed the suggestions to use ML API v2 and fixed merge conflicts with develop. The URL is still hardcoded, but I added a TODO comment to remind us to change it later.

As there are no topics in v2, I have repurposed the chips to show the similarity score of each suggested investigation. The colour intensity of the chip depends on the relative score of each suggestion (i.e. the highest score has the highest intensity):

I also changed the listItems to listItemButtons so that the whole entry is clickable, not just the text. The button is disabled if an entry doesn't have a doi:

Let me know if we want to keep these changes, or if I should revert to the original.

{field.content(data[0] as Investigation)}
</ShortInfoValue>
</ArrowTooltip>
z
Copy link
Member

Choose a reason for hiding this comment

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

Remove this random z character

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought the z character was intentional 😅

Copy link
Member

@louise-davies louise-davies left a comment

Choose a reason for hiding this comment

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

I think I prefer the look of the links - it makes it much clearer that they're clickable and that they're links to other web pages. Theoretically, you could keep the whole button clickable and just make the text "look" like a link, but tbh I think it's just easier to have them as links.

But otherwise, I really like the chips to show the similarity score and the functionality works well :)

@kaperoo
Copy link
Contributor

kaperoo commented Oct 6, 2023

I think I prefer the look of the links - it makes it much clearer that they're clickable and that they're links to other web pages. Theoretically, you could keep the whole button clickable and just make the text "look" like a link, but tbh I think it's just easier to have them as links.

But otherwise, I really like the chips to show the similarity score and the functionality works well :)

Changed the buttons back to links and removed the random 'z' in extra info.

@louise-davies
Copy link
Member

@VKTB this is ready from our end again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datagateway-dataview Issues corresponding to the dataview component. This component supersedes datagateway-table enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add suggested/similar investigations list to investigation landing pages
4 participants