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

Add a directory navigation component #4473

Merged
merged 43 commits into from
Sep 19, 2024

Conversation

cstns
Copy link
Contributor

@cstns cstns commented Sep 10, 2024

Description

Added a directory navigation component that resembles the provided mockup
Simplified frontend file navigation as it had too many moving parts, left the breadcrumbs object as the single source of truth

Related Issue(s)

closes #4449

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Includes a DB migration? -> add the area:migration label

… the breadcrumbs should be ignored due to how the relative path is resolved
…ate after successful visibility update in order to not confuse users
@cstns cstns self-assigned this Sep 10, 2024
@cstns
Copy link
Contributor Author

cstns commented Sep 10, 2024

Mobile is not yet optimized

Copy link

codecov bot commented Sep 10, 2024

Codecov Report

Attention: Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.13%. Comparing base (6a1f73e) to head (82b5128).
Report is 72 commits behind head on main.

Files with missing lines Patch % Lines
forge/ee/routes/staticAssets/index.js 90.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4473   +/-   ##
=======================================
  Coverage   78.12%   78.13%           
=======================================
  Files         296      296           
  Lines       14125    14135   +10     
  Branches     3189     3192    +3     
=======================================
+ Hits        11035    11044    +9     
- Misses       3090     3091    +1     
Flag Coverage Δ
backend 78.13% <90.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…component' into 4449_add-a-directory-navigation-component
…nto 4461_add-file-asset-service-visibility-selector
…bility-selector' into 4461_add-file-asset-service-visibility-selector
@joepavitt joepavitt changed the title 4449 add a directory navigation component Add a directory navigation component Sep 17, 2024
Copy link
Contributor

@joepavitt joepavitt left a comment

Choose a reason for hiding this comment

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

Functionally, excellent and intuitive. Few minor pieces:

Changes I've made

  • Add gap between the "copy" button and the file path for better readability.

Copying Folder Paths

I don't believe there is any value in being able to copy the File Path for a Folder, it also clashes with the row-click which then nests into that folder too. I vote not showing the "copy" button (and functionality) for folder rows.

Screenshot 2024-09-17 at 15 22 57

Inconsistent Highlighting

If you mouseover on the file path for a file, it highlight blue, but isn't clickable, only the "copy" button is. It would be nice to have the text clickable too, which triggers the copy logic.

@joepavitt
Copy link
Contributor

joepavitt commented Sep 18, 2024

Making the ff-text-copier component then broke the extra gap/padding I'd added in defined on ff-row-file.

Update: Just pushed the fix

@joepavitt
Copy link
Contributor

Approved subjects to tests passing one final time - all looks good.

@cstns please make sure we get a changelog entry and updates in the relevant documentation

@cstns
Copy link
Contributor Author

cstns commented Sep 18, 2024

Created FlowFuse/website#2587 for the changelog entry, will tackle both changelog & docs first thing tomorrow

@cstns cstns merged commit 191a309 into main Sep 19, 2024
14 checks passed
@cstns cstns deleted the 4449_add-a-directory-navigation-component branch September 19, 2024 10:29
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.

Add a directory navigation component
3 participants