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

#1665. Improve Package Selection #1687

Merged
merged 26 commits into from
Jun 12, 2023

Conversation

mike-winberry
Copy link
Contributor

@mike-winberry mike-winberry commented May 9, 2023

Is your feature request related to a problem? Please describe.

Our package selection workflow in the UI is a bit slow due to searching the entire home directory at one time.

Describe the solution you'd like

Recommend the following changes:

For 'init' package deployment

  • in the selection window, only show init packages that exist in the three places that Zarf searches for them:
    • Current Working Directory
    • Zarf Binary Directory
    • Home .zarf-cache folder
    • Only show init packages that match the current build version.

Regular Package Deployment

For 'regular' packages - the UX flow proceeds in two parts:

  • Search # 1.

    • Search and show packages located in the current working directory w/o subdirectories (matching the tab behavior on zarf package deploy)
    • If packages are found show in the table and add a expand search button to table that executes search # 2
    • If no packages are found - show no package found error message (gives the user option to cancel deployment or proceed to part # 2
  • search part # 2

    • Search and show packages located in the entire home directory
    • Exclude trash
    • Exclude hidden folders
  • For both workflows, increase the height of the package selection table to show more packages at once

Design File

https://www.figma.com/file/MUItIzpzLBLuIyt225Bwgl/Zarf-Web-Ui?type=design&node-id=1544%3A56348&t=JeB5xVvOb7lcP6ZB-1

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

… update FindInitPackage to use the version name and to look in execution, cache directory, and the current working directory. Update io.go with a FileList method that reads a directory and returns a list of filepaths matched against an optional regex.
@mike-winberry mike-winberry linked an issue May 9, 2023 that may be closed by this pull request
4 tasks
@netlify
Copy link

netlify bot commented May 9, 2023

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 4b02f4e
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/6487a66388bfc50008d35740

…er for packages with the /packages/explore route.
src/pkg/message/message.go Fixed Show fixed Hide fixed
@mike-winberry mike-winberry requested a review from Madeline-UX May 10, 2023 19:39
@mike-winberry
Copy link
Contributor Author

OS File Picker will not work since it is really an upload input and does not reveal the local systems file structure (https://html.spec.whatwg.org/multipage/input.html#fakepath-srsly) leaving options:

  • Education
  • Roll your own file explorer
  • Be a little slow.

Copy link
Contributor

@Madeline-UX Madeline-UX left a comment

Choose a reason for hiding this comment

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

Reviewed the solution that @mike-winberry created for the system browser proposed by the team.

Given the user of this feature is Ashton we do not assume this persona is competent with CLI tool. After reviewing the issue, I do not agree with or approve of this UX flow.
Reasons:

  • unnecessarily complex - User clicking through a file system requires more cognitive load than waiting for a search to populate. User must remember or spend time searching through folders
  • UX flow of searching file system may imply to some users that a file is being uploaded.
  • UI is not consistent with system browser - can lead to confusion for user.
  • Does not explain or provide reason for why Zarf only searches local system
  • Why would we require a user to look for something we can just show them.

@Racer159 Requesting this issue be put on hold until UX design is addressed:

Options to explore

  1. Keep MVP as is until we can gather data on observable user behavior (optimize for speed)
  2. Error message that asks if user would like to search entire system post a local search
  3. Error message telling user to move file to working directory and hope they know how

@Racer159
Copy link
Contributor

Reviewed the solution that @mike-winberry created for the system browser proposed by the team.

Given the user of this feature is Ashton we do not assume this persona is competent with CLI tool. After reviewing the issue, I do not agree with or approve of this UX flow. Reasons:

* unnecessarily complex - User clicking through a file system requires more cognitive load than waiting for a search to populate. User must remember or spend time searching through folders

* UX flow of searching file system may imply to some users that a file is being uploaded.

* UI is not consistent with system browser - can lead to confusion for user.

* Does not explain or provide reason for why Zarf only searches local system

* Why would we require a user to look for something we can just show them.

@Racer159 Requesting this issue be put on hold until UX design is addressed:

Options to explore

1. Keep MVP as is until we can gather data on observable user behavior (optimize for speed)

2. Error message that asks if user would like to search entire system post a local search

3. Error message telling user to move file to working directory and hope they know how

Pushed it back - of these I think 1 and 2 are maybe the best - testing it again it is still a bit slow even with local dir packages (I have more than the average bear but it takes several seconds to return the package information on my system).

mike-winberry and others added 14 commits May 11, 2023 22:42
## Related Issue 
Fixes #1686 
<!-- or -->
Relates to #

## Type of change

- [x] UI Tweak

Co-authored-by: razzle <[email protected]>
Co-authored-by: Wayne Starr <[email protected]>
## Description
#800 



## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [ ] [Contributor Guide
Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow)
followed

---------

Co-authored-by: Wayne Starr <[email protected]>
Co-authored-by: Wayne Starr <[email protected]>
…ts as they are found and open a dialog if not. TODO add recursive file streaming and the expanded search.
…query param init: boolean and recursively streams packages from the users home folder (init packages when init = true). Update utils io.go with IsTrashBin method that takes in a directory path and checks if it is the os trash bin. Updat the ui server with new packages/find/stream routes. Update frontend api with new routes and method for recursively streaming packages. TODO: add the expanded search button and logic.. Handle warnings for init packages that dont match the executable version.
…eList; Extracted hidded directory check into IsHidden utility method; Removed unused FileList method. Update all references to RecursiveFileList references to nolonger have the unused flags. Update api/packages find.go: Removed the initPackagesPattern and replaced references with currentInitPattern; Update recursivePackageStream to ignore hidden directories as well as the Trash folder.
…rch again after expanded search has been run.
@mike-winberry mike-winberry marked this pull request as ready for review June 5, 2023 17:31
@mike-winberry
Copy link
Contributor Author

mike-winberry commented Jun 5, 2023

  • Expanded search when no packages found
Screen.Recording.2023-06-05.at.10.34.46.AM.mov
  • Package found and expand search
Screen.Recording.2023-06-05.at.10.35.30.AM.mov
  • No packages found
Screen.Recording.2023-06-05.at.10.36.17.AM.mov

@mike-winberry mike-winberry requested a review from Madeline-UX June 6, 2023 20:51
Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

Overall much better UX, just a few small things

src/internal/api/packages/find.go Outdated Show resolved Hide resolved
src/internal/api/packages/find.go Outdated Show resolved Hide resolved
src/pkg/utils/io.go Outdated Show resolved Hide resolved
src/pkg/utils/io.go Outdated Show resolved Hide resolved
src/pkg/utils/io.go Outdated Show resolved Hide resolved
src/internal/api/packages/find.go Outdated Show resolved Hide resolved
@mike-winberry mike-winberry requested a review from Racer159 June 8, 2023 04:23
Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

skipHidden should be false on Files (was likely a bug if it wasn't before)

src/internal/packager/template/yaml.go Outdated Show resolved Hide resolved
src/pkg/packager/create.go Outdated Show resolved Hide resolved
src/pkg/packager/deploy.go Outdated Show resolved Hide resolved
@mike-winberry mike-winberry requested a review from Racer159 June 12, 2023 19:04
Madeline-UX
Madeline-UX previously approved these changes Jun 12, 2023
Racer159
Racer159 previously approved these changes Jun 12, 2023
Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

lgtm

@Racer159 Racer159 enabled auto-merge (squash) June 12, 2023 19:11
@mike-winberry mike-winberry force-pushed the 1665-improve-package-selection-web-ui-workflow branch from 281cd6b to 1ab7d49 Compare June 12, 2023 22:45
@mike-winberry mike-winberry requested a review from a team as a code owner June 12, 2023 22:45
@mike-winberry mike-winberry force-pushed the 1665-improve-package-selection-web-ui-workflow branch 5 times, most recently from 3f55d3a to b69f7c5 Compare June 12, 2023 23:08
@mike-winberry mike-winberry dismissed stale reviews from Madeline-UX and Racer159 June 12, 2023 23:12

The merge-base changed after approval.

@Racer159 Racer159 merged commit 77141b7 into main Jun 12, 2023
@Racer159 Racer159 deleted the 1665-improve-package-selection-web-ui-workflow branch June 12, 2023 23:45
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.

Improve package selection Web UI workflow
3 participants