-
Notifications
You must be signed in to change notification settings - Fork 358
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
refactor: [M3-7544, M3-7552] - ErrorState and FileUploader v7 stories #9982
refactor: [M3-7544, M3-7552] - ErrorState and FileUploader v7 stories #9982
Conversation
@@ -1,4 +1,5 @@ | |||
import { getObjectURL } from '@linode/api-v4/lib/object-storage'; | |||
import { AxiosProgressEvent } from 'axios'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved FileUploader and ObjectUploader into one folder underneath the Components package for organization - both stories were housed under components and they're both file uploaders
If we prefer ObjectUploader
to stay underneath features/ObjectStorage
I can undo this change. Otherwise, would appreciate name suggestions for the overarching package besides Uploaders
😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine! I also like it here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! those are not simple stories. The uploaders are tricky so let me know if you want to pair up on those.
Left a few comments to clean things up and improve functionality
packages/manager/src/components/ErrorState/ErrorState.stories.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/components/Uploaders/FileUploader/FileUploader.stories.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any way we can make this story "work" as in I can at least place an artifact in the drop zone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm seems like this component is actually named inaccurately and should be 'Image Uploader' - I'm able to upload images/.gz files, but nothing else
I'm able to put in files for the ObjectUploader, though there's always an error (likely because we're not connected to an actual bucket?).
both behaviors match prod. Gonna rename FileUploader.tsx to ImageUploader
@@ -1,4 +1,5 @@ | |||
import { getObjectURL } from '@linode/api-v4/lib/object-storage'; | |||
import { AxiosProgressEvent } from 'axios'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine! I also like it here
packages/manager/src/features/ObjectStorage/BucketDetail/BucketDetail.tsx
Outdated
Show resolved
Hide resolved
Coverage Report: ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Confirmed Object Storage and Image uploads still worked as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for address feedback. This looks good to go! 🚢
Description 📝
How to test 🧪
Verification steps
As an Author I have considered 🤔
Check all that apply