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.

Sign up for GitHub

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

Jump to bottom

Display synonyms node when hovering #4704

Merged
merged 15 commits into from
May 21, 2024
Merged

Display synonyms node when hovering #4704

merged 15 commits into from
May 21, 2024

Conversation

CarolineDenis
Copy link
Contributor

@CarolineDenis CarolineDenis commented Mar 29, 2024 •
edited
Loading

Fixes #407

Checklist

  • Self-review the PR after opening it to make sure the changes look good
    and self-explanatory (or properly documented)
  • Add automated tests
  • Add relevant issue to release milestone

Testing instructions

  • go to any tree
  • hover over a node that has synonyms
  • verify that the synonyms are being displayed

maxpatiiuk
maxpatiiuk requested changes Mar 30, 2024
View reviewed changes
Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

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

🔻

specifyweb/frontend/js_src/lib/components/TreeView/Row.tsx Outdated Show resolved Hide resolved
specifyweb/frontend/js_src/lib/components/TreeView/Row.tsx Outdated Show resolved Hide resolved
@CarolineDenis
Copy link
Contributor Author

NOTES:
Will try to do something on the back too

maxpatiiuk
maxpatiiuk approved these changes Apr 5, 2024
View reviewed changes
Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

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

Pushed code that resolves these issues
Also, refactored fetchRows definition a bit to reduce duplication
And, added tests while I was at it

@CarolineDenis CarolineDenis marked this pull request as ready for review April 5, 2024 13:53
@CarolineDenis CarolineDenis requested a review from a team April 5, 2024 13:53
@CarolineDenis CarolineDenis added this to the 7.9.6 milestone Apr 16, 2024
emenslin
emenslin requested changes May 8, 2024
View reviewed changes
Copy link
Collaborator

@emenslin emenslin left a comment

Choose a reason for hiding this comment

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

Testing instructions

  • go to any tree
  • hover over a node that has synonyms
  • verify that the synonyms are being displayed

Looks good, when hovering it shows both synonyms and preferred taxon
Screenshot 2024-05-08 090131
Screenshot 2024-05-08 090145

The only issue is that even when there are no synonyms for a taxon the tooltip still shows, I think if there is no synonym/preferred taxon then there shouldn't be a tooltip.
Screenshot 2024-05-08 090427

@CarolineDenis CarolineDenis requested review from emenslin and a team May 9, 2024 16:34
emenslin
emenslin approved these changes May 13, 2024
View reviewed changes
Copy link
Collaborator

@emenslin emenslin left a comment

Choose a reason for hiding this comment

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

Testing instructions

  • go to any tree
  • hover over a node that has synonyms
  • verify that the synonyms are being displayed

Looks good, synonym only displayed if there are actually synonyms.
image

@emenslin emenslin requested a review from a team May 13, 2024 13:51
combs-a
combs-a approved these changes May 13, 2024
View reviewed changes
Copy link
Collaborator

@combs-a combs-a left a comment

Choose a reason for hiding this comment

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

Testing instructions

  • go to any tree
  • hover over a node that has synonyms
  • verify that the synonyms are being displayed

image
image

Looking good, both hovers work.

@CarolineDenis CarolineDenis merged commit 2f89275 into production May 21, 2024
9 checks passed
@CarolineDenis CarolineDenis deleted the issue-407 branch May 21, 2024 16:41
@sharadsw sharadsw mentioned this pull request May 23, 2024
Merged
5 tasks
This was referenced Jul 3, 2024
Closed
Closed
This was referenced Jul 18, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emenslin emenslin emenslin approved these changes

@combs-a combs-a combs-a approved these changes

@maxpatiiuk maxpatiiuk maxpatiiuk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
General Tester Board
Status: ✅Done
Milestone
7.9.6
Development

Successfully merging this pull request may close these issues.

Synonyms should be shown when hovering over a node in a tree
4 participants
@CarolineDenis @maxpatiiuk @combs-a @emenslin