Skip to content

Conversation

@ronronscelestes
Copy link
Contributor

@ronronscelestes ronronscelestes commented Jul 31, 2022

What

Breadcrumbs feature for Media Library folders & MediaLibraryInput modals

How to test it

Navigate into nested folders in the Media Library
Use Breadcrumbs to navigate into previous folders

Snapshot

image

image

@codecov
Copy link

codecov bot commented Jul 31, 2022

Codecov Report

Merging #13917 (d99e8d6) into master (25d178d) will increase coverage by 0.11%.
The diff coverage is 96.10%.

❗ Current head d99e8d6 differs from pull request most recent head 7d31f22. Consider uploading reports for the commit 7d31f22 to get more accurate results

@@            Coverage Diff             @@
##           master   #13917      +/-   ##
==========================================
+ Coverage   55.26%   55.38%   +0.11%     
==========================================
  Files        1253     1259       +6     
  Lines       31593    31684      +91     
  Branches     5706     5732      +26     
==========================================
+ Hits        17460    17547      +87     
- Misses      12319    12323       +4     
  Partials     1814     1814              
Flag Coverage Δ
front 58.12% <96.10%> (+0.15%) ⬆️
unit 48.65% <ø> (ø)

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

Impacted Files Coverage Δ
...in/src/components/BulkMoveDialog/BulkMoveDialog.js 90.00% <ø> (ø)
...load/admin/src/components/EditAssetDialog/index.js 73.86% <ø> (ø)
...rc/components/EditFolderDialog/EditFolderDialog.js 86.17% <ø> (ø)
...src/components/Breadcrumbs/CrumbSimpleMenuAsync.js 88.23% <88.23%> (ø)
...min/src/components/AssetDialog/BrowseStep/index.js 93.13% <91.66%> (+0.50%) ⬆️
...e/upload/admin/src/components/AssetDialog/index.js 74.69% <100.00%> (ø)
...ad/admin/src/components/Breadcrumbs/Breadcrumbs.js 100.00% <100.00%> (ø)
...e/upload/admin/src/components/Breadcrumbs/index.js 100.00% <100.00%> (ø)
packages/core/upload/admin/src/constants.js 100.00% <100.00%> (ø)
packages/core/upload/admin/src/hooks/useFolder.js 100.00% <100.00%> (ø)
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ronronscelestes ronronscelestes added source: core:upload Source is core/upload package pr: feature This PR adds a new feature labels Jul 31, 2022
@ronronscelestes ronronscelestes added this to the 4.3.3 milestone Aug 4, 2022
@ronronscelestes ronronscelestes marked this pull request as ready for review August 4, 2022 09:12
MarionLemaire
MarionLemaire previously approved these changes Aug 4, 2022
Copy link
Contributor

@MarionLemaire MarionLemaire left a comment

Choose a reason for hiding this comment

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

Redirections tested on both Media Library section and Upload modal, on 10 nested levels of folders, using Getstarted : OK to me (to go to nested folder and to root).

Breadcrumbs display tested on Upload modal using Getstarted on 10 levels of nested folders ; OK.
image

Breadcrumbs display tested on Chrome using Getstarted on 10 levels of nested folders : OK.
image

Design QA done by @maevalienard : OK too.

Copy link
Contributor

@gu-stav gu-stav left a comment

Choose a reason for hiding this comment

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

Added a few minor comments: nothing major. I've tested it and think it works very well. Good job on that.

What I am not happy with is the initial view of the media library and the modal: I find "Media Library" repetitive in the ML and it makes the modal very noisy. Similar to the back button: what do you think about hiding the breadcrumbs, if currentFolder === null?

Screenshot 2022-08-04 at 11 27 00

Screenshot 2022-08-04 at 11 25 14

/cc @maevalienard

@maevalienard
Copy link

@gu-stav That would work to me, I didn't know it was technically feasible

@ronronscelestes
Copy link
Contributor Author

@gu-stav @maevalienard @MarionLemaire
integrated: Breadcrumbs will not show if the user is at the root level of the Media Library

Copy link
Contributor

@MarionLemaire MarionLemaire left a comment

Choose a reason for hiding this comment

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

Tested on Gestarted using Chrome :

For Media Libary section : when on root, breadcrumb is not displayed. As soon as you get to a nested folder, breadcrumb appears. Redirections are still OK. Tested on 10 levels of nesting.

Same thing for Upload modal.

Ready to release !

Copy link
Contributor

@gu-stav gu-stav 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 to me. Let's get it on the road! 🚢

Btw: thanks for getting rid of the title truncation. That's a big win!

@strapi-bot
Copy link

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/media-library-folders-beta-is-live/19515/22

@ronronscelestes ronronscelestes merged commit c039852 into master Aug 5, 2022
@ronronscelestes ronronscelestes deleted the features/folder-breadcrumbs branch August 5, 2022 10:56
@pwizla pwizla added the flag: documentation This PR requires a documentation update label Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flag: documentation This PR requires a documentation update pr: feature This PR adds a new feature source: core:upload Source is core/upload package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants