autoOpenFileEditor option is ignored (broken?) past @uppy/dashboard 3.7.4 and @uppy/react about uppy HOT 16 OPEN

I just opened the PR for the config option. I started working on events and it's a bit more tangled than I was comfortable with. In my quick review it looks like it would benefit from a bit of untangling and reworking that I'd consider breaking changes.

Current events:

• dashboard:file-edit-start fires only when the meta editor starts because it's part of the toggleFileCard method
• dashboard:file-edit-complete fires only when the image editor ends

The wording for both suggests image editor, just like the autoOpenfileEditor, the language suggests the same things, but they are describing different events.

Rather than changing these events, which will cause breaking changes, can we consider adding three new pairs of events that are more specific and accurate?

• dashboard:meta-editor-start should fire specifically when editing metadata
• dashboard:meta-editor-complete should fire specifically when done editing metadata, ideally with an argument indicating whether it was submitted or cancelled.
• dashboard:image-editor-start should fire specifically when editing the image
• dashboard:image-editor-end should fire specifically when done editing the image, ideally with an argument indicating whether it was submitted or cancelled
• dashboard:editor-start should fire when the card first appears
• dashboard:editor-end should fire when the card closes

Then leave the existing events alone, possibly deprecate them and stop using them in a future major release, RIP. I'll also add that I am currently using dashboard:file-edit-complete to immediately save changes once editing is complete, so I have a personal stake in not changing in a patch release or it will break my app. 🙏 🙏 🙏

from uppy.

In my case, I use use events to control the behavior:

    // from within useImageUploadEdit, a React Hook

    newUppy.on('file-editor:cancel', () => {
      onCancel();
    });

    newUppy.on('file-editor:complete', (updatedFileMeta) => {
      onEditComplete(updatedFileMeta.data);
    });

This will not help @jbpros because of the Metadata requirement; however, if new events wind up being added then perhaps they will provide the tools needed to control each step of the process.

from uppy.

Hi @subvertallchris!

The behaviour you are describing is not a bug, we intentionally changed the way autoOpenFileEditor works in #4939:

• if you have meta fields enabled, uppy will automatically open the meta fields form;
• if you don't have meta fields enabled, uppy will automatically open the image editor.

This introduces the "file list ⟷ metaEditor ⟷ imageEditor" flow, which makes it easier for the user to understand what's going on.

it's a major breaking change as I rely on this behavior to enforce aspect ratios of many images

Your users can still click on "Edit Image" to apply the aspect ratio, does that not work for you for some reason?

from uppy.

Hi, thanks for the reply. This really looks to me like a breaking change that warrants a major version update. It certainly adds new behavior that justifies a minor version change and not a patch version change. Given that the same exact user code changes the output between versions, it presents to me as a breaking change.

It also introduces confusing and inconsistent behavior. The option is autoOpenFileEditor but the file-editor:start event does not emit until you click that "Edit image" button, which suggests that either the option takes you to the wrong place, the event is fired at the wrong time, or the event name is wrong. The change of behavior based on the presence of a seemingly unrelated field metaFields adds to confusion.

For now, I'm able to get the expected workflow by removing our metaFields. But if we were using that field, it would be a problem. We rely on the image editor as a required step to enforce aspect ratios, so it should be the first thing a user does after selecting a file and impossible for them to get wrong.

If I might make a recommendation, there are a few ways you could improve this behavior.

• A new option, autoOpenMetaEditor that is considered in the private method openFileEditorWhenFilesAdded.
• An option that will control the autoOpeFileEditor behavior. Perhaps autoOpenView: “meta” | “editor” option passed during plugin initialization. Defaults can be what you think are best but it would give users the ability to preserve expected behavior.
• A new event that indicates when the meta editor is opened and then make it easy to programatically switch between views. Working around the new behavior would require some added state to determine if it's auto-open but that could be handled on the user's side, I guess.

Again, thank for your review and your work on this library. It really is great. I hope you will consider this and review this change some more.

from uppy.

The option is autoOpenFileEditor but the file-editor:start event does not emit until you click that "Edit image" button, which suggests that either the option takes you to the wrong place, the event is fired at the wrong time, or the event name is wrong.

You're right that the event name is confusing.

We have two editors: meta editor and image editor.
When the image editor opens, it fires the event (file-editor:start) [It's called file-editor because we could potentially have editors for many types of files, not just the image editor].
When the meta editor opens, it doesn't fire any events.

I agree it would be more consistent if meta-editor:start event would fire.

Perhaps autoOpenView: “meta” | “editor” option passed during plugin initialization.

We did consider exactly this option. It still sounds pretty good to me, but I'm not convinced a lot of people have both meta and image editors enabled, and yet want to jump directly to the image editor first.

In your case it sounds like your issue was resolved by removing the meta fields, so I'm closing this issue.
If anyone else is having the situation that they have both the meta editor and the image editor enabled, and yet want to automatically jump straight to the image editor when they use the autoOpenFileEditor option - please reopen.

from uppy.

If it was considered and you’ve already heard from one very passionate user (me) who invested time troubleshooting and reporting, it is likely that there are others out there who have not yet opened issues directly with you. Providing a path to maintain the existing behavior seems like the bare minimum here.

I am very unhappy with this change and disappointed that you are closing the issue. A change to output without changing inputs can reasonably be considered a breaking change. I hope you will consider improving this. Relevant xkcd: https://xkcd.com/1172

from uppy.

from uppy.

Uh sorry, didn't mean to close the issue that's still of relevance to you!

From what I hear then, you would, ideally, like to have the meta editor present - it's just so that the ability to jump directly to the image editor is more important to you, and you're willing to sacrifice the meta editor.

Your use case originates from the fact that you need to enforce the aspect ratio, which makes it necessary for the user to visit the image editor. Which I think is a valid/widespread use case & it warrants the introduction of autoOpenView: "meta" | "editor" 👍

I’ll also add that I’m happy to provide a pull request to improve some of this behavior if you’ll accept it.

Yep, the PR is welcome! autoOpenView: "meta" | "editor" + "meta-editor:start" event are both desired.

from uppy.

from uppy.

Hey -- new Uppy user here! I'm evaluating Uppy and thought my use case would be of interest to you all, relevant to this issue.

My need is to let users upload a square picture from their webcam or local file to be used as their avatars. My approach is to use autoOpenFileEditor and tell cropper to force an aspectRatio of 1 so that people can fine-tune their square profile picture. That works well.

They also need to tell if the picture can be displayed publicly. I added a checkbox meta field for that.

Because of the current behaviour highlighted here, that prevents the file editor from opening automatically and the aspect ratio of the picture is no longer enforced. Users are not automatically prompted to move/resize their squared picture and are able to upload pictures in any aspect ratio.

Being new to Uppy, I might be missing a completely different/better way of achieving this, though.

So here is my use case, I hope that's helpful in some way. I'm looking forward to that PR!

from uppy.

@jbpros, thank you for your report! Good to know that in both of these cases it's aspect-ratio enforcement related.
Interesting that you manage to depend on it - if the user clicks "CANCEL", the aspect ratio won't be enforced. I guess you're hiding the "CANCEL" button with css?

from uppy.

@jbpros, thank you for your report! Good to know that in both of these cases it's aspect-ratio enforcement related. Interesting that you manage to depend on it - if the user clicks "CANCEL", the aspect ratio won't be enforced. I guess you're hiding the "CANCEL" button with css?

Oh, I missed that cancel link on the editor view! As I said, I'm new to Uppy :) So yes, I'll try hiding it with CSS, thanks for bringing it up.

Regarding the metadata, I think I'm going to take it outside of the upload process for now, as a separate step and a default value on the server. Obviously not the best in terms of UX.

from uppy.

I found the CSS trick a bit too much of a hack and it was interfering with legit cases of using the cancel link. Here is what I came up with instead:

.on('file-editor:cancel', () => {
  const file = uppy.getFiles()[0]
  const img = new Image()

  img.src = URL.createObjectURL(file.data)
  img.onload = () => {
    // the user hit cancel before cropping to 1:1 if the height and width don't match
    // let's consider they want to start over
    if (img.width !== img.height) uppy.removeFile(file.id)
    URL.revokeObjectURL(img.src)
  }
})

from uppy.