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

[BI-1414] - Improve table loading message #205

Merged
merged 2 commits into from
Mar 16, 2022
Merged

[BI-1414] - Improve table loading message #205

merged 2 commits into from
Mar 16, 2022

Conversation

HMS17
Copy link
Contributor

@HMS17 HMS17 commented Mar 10, 2022 •
edited
Loading

Description

Story: BI-1414 - Improve table loading message

Updated both SidePanelTable.vue and ExpandableTable.vue to take in a loading prop and use it for logic re: displaying a no entries message.

Dependencies

bi-web/develop

Testing

  • Open Ontology Table with no entries - no entries message displays only after loading complete
  • Open Ontology Table with entries - no entries message never displays
  • Open Germplasm Table with no entries - no entries message displays only after loading complete
  • Open Germplasm Table with entries - no entries message never displays

Checklist:

  • I have performed a self-review of my own code
  • I have tested my code and ensured it meets the acceptance criteria of the story
  • [n/a] I have create/modified unit tests to cover this change
  • [n/a] I have commented my code, particularly in hard-to-understand areas
  • [n/a] I have made corresponding changes to documentation
  • I have run TAF: <link to TAF run>

@HMS17 HMS17 marked this pull request as ready for review March 10, 2022 19:51
@HMS17 HMS17 requested review from a team, davedrp and ctucker3 and removed request for a team March 10, 2022 19:51
ctucker3
ctucker3 approved these changes Mar 14, 2022
View reviewed changes
Copy link
Contributor

@ctucker3 ctucker3 left a comment

Choose a reason for hiding this comment

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

I requested changes on BI-1404 which this one seems to be based on. So this should wait to be merged until this changes are merged.

ctucker3
ctucker3 suggested changes Mar 14, 2022
View reviewed changes
Copy link
Contributor

@ctucker3 ctucker3 left a comment

Choose a reason for hiding this comment

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

Small change

src/components/ontology/OntologyTable.vue Outdated
}
}).catch((error) => {
// Display error that traits cannot be loaded
this.$emit('show-error-notification', 'Error while trying to load traits');
this.traitsLoading = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as on 1404, but these this.traitsLoading = false can be put into a finally so they are duplicated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Updated.

@HMS17 HMS17 requested a review from ctucker3 March 14, 2022 20:14
davedrp
davedrp reviewed Mar 14, 2022
View reviewed changes
Copy link
Contributor

@davedrp davedrp left a comment

Choose a reason for hiding this comment

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

happy-path test passed

@HMS17 HMS17 merged commit 491ff65 into develop Mar 16, 2022
@HMS17 HMS17 deleted the feature/BI-1414 branch March 16, 2022 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ctucker3 ctucker3 ctucker3 approved these changes

@davedrp davedrp davedrp approved these changes

Assignees

@davedrp davedrp

Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@HMS17 @davedrp @ctucker3