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.

Sign up for GitHub

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

Jump to bottom

3494 media field revamp #3524

Merged
merged 44 commits into from
Apr 7, 2021
Merged

3494 media field revamp #3524

merged 44 commits into from
Apr 7, 2021

Conversation

konzz
Copy link
Member

@konzz konzz commented Mar 16, 2021

fixes #3494

PR checklist:

  • Update READ.me ?
  • Update API documentation ?

QA checklist:

  • Smoke test the functionality described in the issue
  • Test for side effects
  • UI responsiveness
  • Cross browser testing
  • Code review

@konzz konzz force-pushed the 3494-media-field-revamp branch from fdf54ea to f8154e7 Compare March 21, 2021 22:29
@yacky yacky force-pushed the 3494-media-field-revamp branch from c2c7ce6 to 89b1ace Compare March 26, 2021 09:20
@konzz konzz force-pushed the 3494-media-field-revamp branch from 89b1ace to 33c15d7 Compare March 29, 2021 06:15
vkozinec and others added 27 commits March 30, 2021 11:22
@vkozinec @konzz
Initial modal UI
c80ef71
@yacky @konzz
Open and close media modal
bfc4ce4
@yacky @konzz
show attachments in media modal
e953d79
@konzz
RenderAttachment Component
2996e4c
@konzz
vaultsync storing attachment id instead of url to file
610faea
@yacky @konzz
Use RenderAttachment component in MediaModal
bbe2de6
@yacky @konzz
add onChange and selectedId props to MediaModal
ea405cd
@konzz
Added a migration to populate mimetype on files
c1a367d
@konzz
Remove migration that populates mimetype on files
5d60352
@vkozinec @konzz
Styled media modal items, image and video
402342f
@konzz
updated csv to use attachments when importing image or media fields
b9a7890
@yacky @konzz
add selected image to state
36eb569
@vkozinec @konzz
Styled RenderAttachment component in sidebar
93893ee
@vkozinec @konzz
Added initial media modal test
cc043ed
@yacky @konzz
media field form component
eb2c19d
@yacky @konzz
fix basic modal test for media modal
1cb1981
@yacky @konzz
remove unused variables and functions
dd2ba00
@yacky @konzz
Filter image and media files in media modal. Show selected attachments
826e45f
@yacky @konzz
fix some tests
9718fd2
@konzz
fixed typescript error
3e92cb1
@konzz
deleted deprecated comment
80a751a
@yacky @konzz
fix storeKey and form not working
5a78a6f
@konzz
default empty attachments for when creating a new enity
e21a536
@yacky @konzz
media modal test
0d125dd
@yacky @konzz
fix test describe
ef422da
@konzz
undo changes to csv import and vault sync
3525487
@yacky @konzz
save url of attachment instead of _id
ec9ccc3
@konzz konzz force-pushed the 3494-media-field-revamp branch from b72264c to 3f31c95 Compare March 30, 2021 09:22
@konzz konzz marked this pull request as ready for review March 30, 2021 14:50
mfacar
mfacar requested changes Mar 30, 2021
View reviewed changes
Copy link
Collaborator

@mfacar mfacar left a comment

Choose a reason for hiding this comment

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

@konzz, @vkozinec, @yacky, Nice job! the code looks pretty nice, I just left some comments about small things.

Regarding the Functional Review:

  • @konzz can you please specify the scope of the PR, maybe updating the related issue?.
  • I just tested the edition and visualization of entity properties with type image/media, which I think are the first and second points of issue Image and Media Field revamp (30pts) #3494 still with the URL as the value of the property. These functionalities are working pretty well.
  • I only found two minor issues of UI on Add form url tab
    • Information icon doesn’t show any hint
    • The label of the button should be Add Resource according to the design.

app/react/App/scss/modules/_attachments-modal.scss Outdated Show resolved Hide resolved
app/react/Attachments/components/specs/RenderAttachment.spec.tsx Show resolved Hide resolved
app/react/Documents/components/DocumentSidePanel.js Outdated Show resolved Hide resolved
app/react/Forms/components/MediaField.tsx Outdated Show resolved Hide resolved
app/react/Metadata/components/MediaModal.tsx Outdated Show resolved Hide resolved
app/react/Metadata/components/specs/MediaModal.spec.tsx Outdated Show resolved Hide resolved
app/react/Metadata/components/specs/MediaModal.spec.tsx Show resolved Hide resolved
app/react/Metadata/containers/FormatMetadata.js Outdated Show resolved Hide resolved
nightmare/suite1/metadataprops.spec.js Outdated Show resolved Hide resolved
app/react/Metadata/components/specs/MediaModal.spec.tsx Show resolved Hide resolved
@mfacar
Copy link
Collaborator

mfacar commented Mar 30, 2021

Also, please can you take a look at the Code Climate issues? Probably some of them could be solved.

@mfacar mfacar self-assigned this Mar 31, 2021
mfacar
mfacar approved these changes Apr 7, 2021
View reviewed changes
Copy link
Collaborator

@mfacar mfacar left a comment

Choose a reason for hiding this comment

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

LGTM, I'll merge the PR as soon as the fixes for the from the web tab be ready

@mfacar mfacar merged commit 0af9aa6 into development Apr 7, 2021
@mfacar mfacar deleted the 3494-media-field-revamp branch April 7, 2021 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yacky yacky yacky left review comments

@vkozinec vkozinec vkozinec left review comments

@mfacar mfacar mfacar approved these changes

Assignees

@mfacar mfacar

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Image and Media Field revamp (30pts)
4 participants
@konzz @mfacar @vkozinec @yacky