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

Abstract attachments into new components #330

Closed
wants to merge 65 commits into from

Conversation

susanm74
Copy link
Contributor

fixes #329

@codecov
Copy link

codecov bot commented May 15, 2023

Codecov Report

Merging #330 (c698985) into main (7633e67) will decrease coverage by 0.22%.
The diff coverage is 74.26%.

@@            Coverage Diff             @@
##             main     #330      +/-   ##
==========================================
- Coverage   80.10%   79.89%   -0.22%     
==========================================
  Files          70       76       +6     
  Lines       18593    19531     +938     
  Branches      625      631       +6     
==========================================
+ Hits        14894    15604     +710     
- Misses       3693     3920     +227     
- Partials        6        7       +1     
Impacted Files Coverage Δ
src/attachments/attachments.ts 39.86% <39.86%> (ø)
src/attachments/AttachmentsUploader.ts 59.65% <59.65%> (ø)
src/attachments/AttachmentsDropZone.ts 76.51% <76.51%> (ø)
src/compose/Compose.ts 87.62% <77.77%> (+7.96%) ⬆️
src/attachments/AttachmentsPicker.ts 80.67% <80.67%> (ø)
src/imagepicker/ImagePicker.ts 84.98% <84.98%> (ø)
src/attachments/AttachmentsList.ts 90.12% <90.12%> (ø)
src/contacts/ContactChat.ts 86.73% <100.00%> (ø)
src/interfaces.ts 100.00% <100.00%> (ø)
src/utils/index.ts 60.84% <100.00%> (+0.18%) ⬆️
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@susanm74 susanm74 changed the title Abstract attachments into new component Abstract attachments into new components May 15, 2023
susanm74 and others added 25 commits May 15, 2023 21:58
- removed unused styled in AttachmentsList
- added missing closing quotes to drag-dropped and content-changed event handlers
- working, but with what feels like more re-renders than necessary, and so wondering if we can cut down on those
- also wondering if AttachmentsList should be it's own separate component or just implemented as part of whatever component is using the AttachmentsDropZone and AttachmentsUploader components
- updated handleAttachmentsAdded to do one requestUpdate for attachmentsList instead of two separate updates (one for currentAttachments, one for failedAttachments)
- added requestUpdate for attachmentsUploader in handleAttachmentsRemoved
- moved attachment interface from Attachments to AttachmentUploader
- removed attribute: false from failedAttachments properties across all 3 components
- added two image-pickers to demo page, one for avatars, one for logos
- added vector icons for avatars and logos
- added removeIcon property for configuring remove icon
- added uploadIcon property for configuring upload icon
- added default upload_icon and max_attachments to AttachmentsUploader
- reworked uploadFiles to check for when maxAttachments is 1
- reworked uploadFiles to check whether the total of currentAttachments and filesToUpload exceeds maxAttachments and to adjust the filesToUpload to only allow up to maxAttachments (assuming all of the filesToUpload will be successful)
- updated addCurrentAttachment and addFailedAttachment to check for maxAttachments
- updated render to check for maxAttachments and render the input as single vs multiple when 1 vs 1+
…omponents

- updated AttachmentsDropZone slot to remove name attribute
- added maxAttachments attribute to AttachmentsPicker, and now setting maxAttachments in AttachmentsPicker and ImagePicker (instead of AttachmentsUploader)
- fixed AttachmentsUploader single vs multiple input bug by creating render helper getInput()
- reduced of ImagePicker img width and height to match closer to product
- added actions-left styling to ImagePicker
- added "remove image" to ImagePicker getActions() - need to finalize where to place the "remove image" - top right corner vs. on same horizontal plane as "upload image" vs somewhere else
- created a draft Compose2 to start reworking the new attachments components into compose
- removed attachment_avatar and attachment_logo from vector icons enum in favor of using attachment and attachment_image for now
- moved remove icon to inside of div.image-item so as to allow for overlapping of the remove icon in the bottom right corner of the img
- switched from div.actions-left, div.actions-center, and div.actions-right to just div.action-item
…achmentsUploader inner components

- added custom event temba-container-clicked
- added temba-container-clicked event listener to AttachmentsDropZone
- added temba-container-clicked event handler to Compose2
- simplified AttachmentsPicker getActions and replaced div.actions-left, actions-center, and actions-right with div.action-item

- unit test debugging in progress to fix attachments getting mistakenly cleared out when the component is initialized with 1+ attachments
- updated styles to remove inner image-item flexbox styling
…d remove

- added temp dupe updateComponent in compose test file to fix compile errors in contact chat test file
- added configurable property for AttachmentsDropZone drop-mask text
- added configurable property for AttachmentsUploader upload icon text
- worked through AttachmentPicker and ImagePicker happy vs error paths to hone in on pre-upload vs. post-upload display
- adding and updating unit tests in progress
…nd always apply the image-item class stylings
…"${}" -> @click=${}

- added min and max file size to AttachmentsUploader... when maxAttachments=1, an error is returned by the uploader... when maxAttachments=1+, the offending files are removed and the remaining files are uploaded
- added helper method for displaying min max file size errors based on whether one or both are set
- added temba-field wrapper to ImagePicker to display errors and help_text and eventually to use as a widget inside of RP
- updated getImage to display the "attachment upload icon" initially, the attached image when populated, or the "attachment error icon" if something during upload failed or went wrong
- added minFileSize of 2kb and maxFileSize of 500kb to ImagePicker attachments uploader
- added configurable maxFileSize to AttachmentsPicker (same as the AttachmentsUploader by default)
- AttachmentsUploader - added default values for uploadIcon, uploadText, maxAttachments, and maxFileSize properties
- AttachmentsUploader - abstracted pre-upload FileList validation into helper methods
- AttachmentsUploader - added removeInvalidFiles as an optional param to all validation methods (set to true by default) - when set to true, removes invalid files from the list of valid files to upload, when set to false, adds the invalid files to the this.failedAttachments array
- AttachmentsUploader - added validation for equal/square file dimensions for when maxAttachments=1
- AttachmentsUploader - removed getUploadFIleSizeError helper method
- ImagePicker - using a temp maxFileSize of 200 KB
- ImagePicker - using temba-field errors instead of helpText for pre-upload errors
- ImagePicker - removed initial icon display
- added attachments.ts as an attachments-related utils file for upload validation fns
- added handleUploadValidation to AttachmentsPicker to validate for duplicate files, max attachments, and max file size
- added InvalidFile and UploadValidationResult interfaces to AttachmentsUploader
- added validateFiles which gets called before uploadFiles in AttachmentsUploader
- removed check for failedAttachments when displaying uploadText in AttachmentsUploader
-  updated line in Compose2 to fix compile err based on attachments-related changes
- added enum ImageType in ImagePicker for images, avatars, and logos in ImagePicker
- added style classes for avatars and logos in ImagePicker
- added handleUploadValidation to ImagePicker
- ImagePicker getImage updates in progress
- added todo's to Compose2 re: necessity of explicitly declaring the widgetOnly and errors props
…failedAttachments

- Compose2 - removed clearing out of currentAttachments and failedAttachments
- ImagePicker - minor updates to whitespace
- AttachmentsPicker - moved initialization of children's component properties from updated to firstUpdated... removed failedAttachments from updated fn changes check... added deserialize and serialize fn's...
- Compose2 - moved initialization of children's component properties from updated to firstUpdated... removed failedAttachments from updated fn changes check...
- ImagePicker - moved initialization of children's component properties from updated to firstUpdated... removed failedAttachments from updated fn changes check... added deserialize and serialize fn's...
- added initial ImagePicker unit testing
- replace Compose with Compose2 (renamed to Compose) and updated unit testing

- todo - validation testing for Compose, AttachmentsPicker, and ImagePicker
- todo - fix bug where screenshot not displaying ImagePicker success file (just need to update url to point to test-asset)
- todo - fix bug where screenshot not displaying temba-field errors
- moved upload_endpoint from attachments to AttachmentsUploader
- Compose - removed extra Attachment interface
…hots

- removed image picker temp hack spoof code
- added attachments uploader unit tests
- moved getValidAttachments and getInvalidAttachments from attachments picker to attachments uploader
@ericnewcomer
Copy link
Member

Didn't get to this today due to form element drama this morning. Will look at it tomorrow though!

- simplified image picker unit test screenshot file names
- added styleMap to pull out ImagePicker .image-item styles from the render template html code
- abstracted imageMargin for reuse across 1) calculating the drop zone width, and 2) styling the  uploaded image (both the missing image or the uploaded one)
@susanm74
Copy link
Contributor Author

@ericnewcomer i'm still getting sorted with my local environment, so i still haven't done some final e2e testing on the compose updates, so although this is ready to review, i'm leaving it marked as draft as it's still not quite ready to merge until i've done that

@ericnewcomer
Copy link
Member

Are you still around that we can look at that today?

@ericnewcomer
Copy link
Member

Had a look through this, I do have some comments, but I'd like to see it in an application before discussing it further.

@github-actions
Copy link

Without activity, this PR will be closed in 14 days.

@github-actions
Copy link

This PR was closed for inactivity.

@github-actions github-actions bot closed this Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Abstract attachments out of compose its own components
2 participants