Skip to content

adds new edit flow to the file block#15515

Closed
draganescu wants to merge 6 commits intomasterfrom
update/new-edit-flow-file
Closed

adds new edit flow to the file block#15515
draganescu wants to merge 6 commits intomasterfrom
update/new-edit-flow-file

Conversation

@draganescu
Copy link
Copy Markdown
Contributor

@draganescu draganescu commented May 8, 2019 •
edited by talldan
Loading

Description

Closes #14795
Also fixes #10521

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

55976214-53a8d700-5c94-11e9-98ee-a41235cc8466

@draganescu draganescu changed the title adds new edit flow adds new edit flow to the file block May 8, 2019
This was referenced May 8, 2019
Closed
Closed
kjellr
kjellr approved these changes May 9, 2019
View reviewed changes
Copy link
Copy Markdown
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.

From a design perspective, this works great!

file

Should get a code review before merge though. 👍

Comment thread packages/core-data/README.md Outdated
kjellr
kjellr suggested changes May 9, 2019
View reviewed changes
Copy link
Copy Markdown
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.

Actually — I'm am seeing the issue @talldan reported in the old thread:

The filename and file link isn't updated when adding or replacing a file using 'Insert from URL' (could just use the url).

@gziolo gziolo added [Block] File Affects the File Block [Type] Enhancement A suggestion for improvement. Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code labels May 10, 2019
@talldan talldan mentioned this pull request May 24, 2019
Closed
@draganescu draganescu force-pushed the update/new-edit-flow-file branch from a9c7167 to cf70e0d Compare May 27, 2019 09:07
@draganescu
Copy link
Copy Markdown
Contributor Author

draganescu commented May 27, 2019

@kjellr I have fixed the issue reported by @talldan and also rebased the PR

@kjellr
Copy link
Copy Markdown
Contributor

kjellr commented May 27, 2019

Thanks! This is looking great. The last issue I'm seeing is that the cancel button is missing for Contributor users:

Screen Shot 2019-05-27 at 11 48 46 AM

(I'm also thinking we should rewrite that description text depending on the user role, but I think that's probably another issue.)

@draganescu draganescu requested a review from ellatrix as a code owner May 28, 2019 20:05
@draganescu
Copy link
Copy Markdown
Contributor Author

draganescu commented May 28, 2019

@kjellr fixed this issue with no cancel link. This fix will also solve #15517 's similar problem because the issue is in the common mediaPlaceholder component, so if this gets merged first the other will display the cancel link too.

@mapk
Copy link
Copy Markdown
Contributor

mapk commented May 28, 2019

I just tested the file block and noticed a weird issue. I'm using FF.

Steps to reproduce:

  1. Add a file block.
  2. Select a file from the Media Library.
  3. After selecting the file, observe that the name is not replacing the placeholder text.

file

@draganescu
Copy link
Copy Markdown
Contributor Author

draganescu commented Jun 4, 2019

@mapk for me it doesn't happen in Firefox 67. What FF do you have?

@draganescu draganescu force-pushed the update/new-edit-flow-file branch from b5ddd48 to 3f464e0 Compare June 4, 2019 21:34
@mapk
Copy link
Copy Markdown
Contributor

mapk commented Jun 10, 2019

@draganescu I just tested again using FF 67.0.1 and it's working fine. I'm not sure which version I had running when I saw that, but I know I just updated today before testing this out.

@draganescu
Copy link
Copy Markdown
Contributor Author

draganescu commented Jul 31, 2019

#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-file branch May 27, 2020 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduth aduth Awaiting requested review from aduth

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@gziolo gziolo Awaiting requested review from gziolo

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

@nerrad nerrad Awaiting requested review from nerrad

@noisysocks noisysocks Awaiting requested review from noisysocks

@notnownikki notnownikki Awaiting requested review from notnownikki

@Soean Soean Awaiting requested review from Soean

@talldan talldan Awaiting requested review from talldan

@youknowriad youknowriad Awaiting requested review from youknowriad

@ellatrix ellatrix Awaiting requested review from ellatrix

1 more reviewer

@kjellr kjellr kjellr requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

[Block] File Affects the File Block Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code [Type] Enhancement A suggestion for improvement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Expand the new replace image flow out to other blocks Add a URL field for File blocks

5 participants

@draganescu @kjellr @mapk @talldan @gziolo