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

adds new edit flow to the media+text block #15517

Closed
wants to merge 8 commits into from

Conversation

draganescu
Copy link
Contributor

Description

Closes #14795

This is a follow up to the image block edit flow update which ports the new flow to all the blocks which have media with an edit state.

How has this been tested?

For now I've only tested locally.

Types of change

New feature (non-breaking change which adds functionality)

Screenshots

media+text

Copy link
Contributor

@kjellr kjellr left a comment

Choose a reason for hiding this comment

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

This is working well for me. The only small visual issue I'm seeing is the issue @talldan noted in the original thread:

Some space is needed between the Media Library and Insert From URL buttons (seems to also apply to other blocks). This does seem to be an issue with the image block as well, so could always fix it after this PR for all blocks:

I agree we could fix this separately if it's not a super-quick global fix.

@gziolo gziolo added [Block] Media & Text Affects the Media & Text Block [Type] Enhancement A suggestion for improvement. labels May 10, 2019
@draganescu draganescu force-pushed the update/new-edit-flow-media-text branch from e05e3cf to 77718ab Compare May 24, 2019 09:04
@draganescu
Copy link
Contributor Author

To have proper spacing between buttons we need a better structure in media-placeholder output b/c doing it via CSS adds a lot of bloat for no reason. Will open a separate PR for that :) @talldan @kjellr

@kjellr
Copy link
Contributor

kjellr commented May 27, 2019

To have proper spacing between buttons we need a better structure in media-placeholder output b/c doing it via CSS adds a lot of bloat for no reason. Will open a separate PR for that :)

Sounds good. This PR is looking pretty good in that case. The only bug I'm seeing now is that while I can make use of the "Insert from URL" option as when logged in as a Contributor, the cancel link doesn't display under the button:

Screen Shot 2019-05-27 at 11 32 13 AM

@mapk
Copy link
Contributor

mapk commented May 28, 2019

I tested this today and it works well. I ran through a few configurations and it was great! Thanks for your work on this @draganescu 💯

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

Hi @draganescu the comment I left on the cover block #15519 (review) also applied here. I think we should have a solution used in both blocks.

@draganescu
Copy link
Contributor Author

#11952 changed direction so closing this as it became irrelevant.

@draganescu draganescu closed this Jul 31, 2019
@youknowriad youknowriad deleted the update/new-edit-flow-media-text branch May 27, 2020 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Media & Text Affects the Media & Text Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expand the new replace image flow out to other blocks
6 participants