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

task/WP-304: Fixed the placeholder text for search to be more accurately represented #905

Merged
merged 4 commits into from
Nov 30, 2023

Conversation

tjgrafft
Copy link
Collaborator

@tjgrafft tjgrafft commented Nov 20, 2023

Overview

The search bar's placeholder text, specifically in Data Files, was not accurately represented based on navigation and possible search results.

  1. Open Data Files.
  2. Initially the search placeholder text might say "Search My Data(Frontera)".
  3. After navigating into folders inside My Data(Frontera), the placeholder text remains the same regardless of file depth/navigation.
  4. The user could mistake the search path for wrong one. For example:
  • If I've navigated to the following file hierarchy: My Data(Frontera) > archive > jobs > 2023-11-20, the current search bar text placeholder would say "Search My Data(Frontera)".
  • But if I try and search with the keyword "archive", I'll get no results.
  • However, if I navigated back to My Data(Frontera) and tried to search the same keyword "archive", I'd get the folder result.

In other words, having the placeholder be the system name the entire time implies I can search the whole system for folders/files regardless of depth/hierarchy--which isn't the case.

To fix this, I've made the placeholder text more accurate.

Related

Changes

  • Added code to check if the current path is homeDir. If user is in the homeDir, the search bar's text placeholder will use the appropriate system name. If the user is not in the homeDir, the current directory name will be displayed for the text placeholder.

Testing

  1. Go to cep.test (make sure your search index is properly set up)
  2. Navigate to 'Data Files'
  3. Select 'My Data(Frontera)' as your system
  4. The initial text placeholder should say 'Search My Data(Frontera)'.
  5. Try searching for a folder or file name you know is inside this system.
  6. Navigate to a folder inside My Data(Frontera) and confirm that the placeholder text is the current directory of the folder you're in.
  7. Try searching for a folder/file that's higher in the hierarchy. It shouldn't return those results since you're deeper in the file tree.
  8. See my examples below in the screenshots to compare.
  9. When testing, make sure that the other Search bar's placeholder text and functionality still work. Site-wide search bar (cep.test/search) and Search bar for job history in "History" tab, for example.

UI

Screenshot 2023-11-20 at 3 13 13 PM

Screenshot 2023-11-20 at 3 14 10 PM

Searching for 'jobs' keyword from My Data(Frontera) directory
Screenshot 2023-11-20 at 3 14 51 PM

Notes

Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Merging #905 (871198d) into main (fc00451) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #905      +/-   ##
==========================================
+ Coverage   63.47%   63.49%   +0.02%     
==========================================
  Files         431      431              
  Lines       12404    12412       +8     
  Branches     2585     2588       +3     
==========================================
+ Hits         7873     7881       +8     
  Misses       4315     4315              
  Partials      216      216              
Flag Coverage Δ
javascript 69.79% <100.00%> (+0.03%) ⬆️
unittests 57.07% <ø> (ø)

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

Files Coverage Δ
...ts/DataFiles/DataFilesListing/DataFilesListing.jsx 90.47% <100.00%> (+2.24%) ⬆️

Copy link
Collaborator

@chandra-tacc chandra-tacc left a comment

Choose a reason for hiding this comment

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

LGTM and also tested. Thanks for adding this feature.

Would it be possible to update DataFilesListing.test.js to add tests to check scenarios related to home dir or not, current dir updates and so on?

Copy link
Collaborator

@chandra-tacc chandra-tacc left a comment

Choose a reason for hiding this comment

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

Thanks for adding tests.

Copy link
Contributor

@edmondsgarrett edmondsgarrett left a comment

Choose a reason for hiding this comment

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

Looks good, nice work!

@chandra-tacc chandra-tacc merged commit 7f26642 into main Nov 30, 2023
6 checks passed
@chandra-tacc chandra-tacc deleted the task/WP-304-search-placeholder-fix branch November 30, 2023 22:06
chandra-tacc pushed a commit that referenced this pull request Dec 8, 2023
…ely represented (#905)

* Fixed the placeholder text for search to be more accurately represented

* adding tests for placeholder text

---------

Co-authored-by: Taylor Grafft <tgrafft@wireless-10-145-54-70.public.utexas.edu>
Co-authored-by: Taylor Grafft <tgrafft@wireless-10-147-30-100.public.utexas.edu>
Co-authored-by: edmondsgarrett <43251554+edmondsgarrett@users.noreply.github.com>
chandra-tacc pushed a commit that referenced this pull request Dec 8, 2023
…ely represented (#905)

* Fixed the placeholder text for search to be more accurately represented

* adding tests for placeholder text

---------

Co-authored-by: Taylor Grafft <tgrafft@wireless-10-145-54-70.public.utexas.edu>
Co-authored-by: Taylor Grafft <tgrafft@wireless-10-147-30-100.public.utexas.edu>
Co-authored-by: edmondsgarrett <43251554+edmondsgarrett@users.noreply.github.com>
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