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

Bug/WP-261: Search bar component - fix js warnings related to button usage #974

Merged
merged 4 commits into from
Oct 14, 2024

Conversation

chandra-tacc
Copy link
Collaborator

@chandra-tacc chandra-tacc commented Sep 25, 2024

Overview

Button has a validation that the children property is a string.

Related

Changes

  • collapse the span into the button and set the test id
  • Ensure the button text is a string
  • Adjust unit tests

Testing

  1. Run this test before and after the fix and ensure there are no warnings: npm run test -- client/src/components/_common/Searchbar/Searchbar.test.jsx
  2. In the UI, go to a data file browser, https://cep.test/workbench/data/tapis/community/cloud.data/corral/tacc/aci/CEP/community/ and search. There should be Back to All Files button link in the top. It should work.

UI

Notes

@chandra-tacc chandra-tacc changed the title Bug/WP-261: Fix js warnings related to button usage Bug/WP-261: Search bar component - fix js warnings related to button usage Sep 25, 2024
Copy link

codecov bot commented Sep 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.09%. Comparing base (f946847) to head (078a9df).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #974   +/-   ##
=======================================
  Coverage   73.09%   73.09%           
=======================================
  Files         531      531           
  Lines       33150    33152    +2     
  Branches     2922     2922           
=======================================
+ Hits        24230    24232    +2     
  Misses       8726     8726           
  Partials      194      194           
Flag Coverage Δ
javascript 76.12% <100.00%> (+<0.01%) ⬆️
unittests 60.12% <ø> (ø)

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

Files with missing lines Coverage Δ
...ent/src/components/_common/Searchbar/Searchbar.jsx 93.75% <100.00%> (+0.07%) ⬆️

@chandra-tacc chandra-tacc marked this pull request as ready for review September 25, 2024 19:00
Copy link
Member

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

Provided button UI has not changed, this is great. I do not know why buttons have internal <span> tags.

Copy link
Contributor

@van-go van-go left a comment

Choose a reason for hiding this comment

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

LGTM. I ran the test in main and this branch. This branch has no warnings.
One note, the test should be:
npm run test -- src/components/_common/Searchbar/Searchbar.test.jsx

@chandra-tacc chandra-tacc merged commit abbe219 into main Oct 14, 2024
6 checks passed
@chandra-tacc chandra-tacc deleted the bug/WP-261 branch October 14, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants