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

Unset Media doesn't work #1427

Closed
guins opened this issue Apr 18, 2018 · 25 comments
Closed

Unset Media doesn't work #1427

guins opened this issue Apr 18, 2018 · 25 comments
Assignees
Labels

Comments

@guins
Copy link

guins commented Apr 18, 2018

Hi, it appears that the "unset" media button (the cross icon when you hover over a preview file) doesn't seem to work.

Setup :

  • Grav install: Grav v1.4.3 & Grav Admin Panel v1.7.4
  • Have a page with several image file fields with multiple attribute set to false
  • Add a image for each file field
  • Save the page
    (It should be all good up to here)

My .yaml file :

header.custom.image1:
    type: file
    label: "Image 1"
    filesize: 3
    destination: 'self@'
    accept:
        - image/*

header.custom.image2:
    type: file
    label: "Image 2"
    filesize: 3
    destination: 'self@'
    accept:
        - image/*

Problem 1 :

  • Unset one of the images using the cross icon when you hover the preview
  • Notice that the drag & drop zone doesn't display any message now
  • Try to upload a new file. You will get the error popin You can not upload any more files.

Problem 2 :

  • Unset one of the images using the cross icon when you hover the preview
  • Notice that the dropdown zone doesn't display any message now
  • Save the page
    Now you should end up with all image file fields, filled with the same image ! (the image is the first filled image field)

@lepascal posted an exemple of this issue here with a repeater field, but it's also true with only 2 image fields.

Temporary Solution 1 - NOT WORKING
I wanted to set my image file fields as required using the validate.required attribute in order to prevent saving the page if an image field is empty, but this is not working. This is a know bug and I added a comment here

Temporary Solution 2 - working, but not ideal
As the problem seems to come from the cross icon only, I got rid of it.
In my theme, I added this piece of code into my custom.js file :

$(function(){
    // Hide existing unset buttons
    $('.dz-unset').css('display', 'none');

    // Prevent click on unset buttons
    // In case the button was added dynamically
    // or the display attribute was overridden
    $(document).on('click', '.dz-unset', function (e) {
        e.preventDefault();
        e.stopImmediatePropagation();
        alert('Sorry, the unset button is not working');
    });
});

Thanks

@lepascal
Copy link

I wanted to set my image file fields as required using the validate.required attribute in order to prevent saving the page if an image field is empty, but this is not working. This is a know bug and I added a comment here

This would not have an effect for me, because every unset action would result in the behaviour described, not only if the page was saved with an empty field.

The second solution is fine for me, though. Thank you! In my case, delete would be the preferred action anyway.

@rhukster
Copy link
Member

unset on file field should be working now.

@rhukster rhukster added fixed and removed evaluating labels May 19, 2018
@dotvhs
Copy link

dotvhs commented Aug 16, 2018

It does a really weird thing for me. I have multiple file fields in my page and if I use Unset on any of them I still cannot upload new image, the error You can not upload any more files. shows up and, what's actually worse, when I save the page to refresh its state, all other file fields will start using a same single file. Am I doing something wrong or it's bugged again?

Not sure if that's understandable, I can write the steps, if needed. For now I will use Delete, but would be great if this could be fixed.

@adamck
Copy link

adamck commented Aug 31, 2018

Having the same issue as @voythas. My workaround for these custom file fields is to remove the unset button from dropzone.

EDIT: in fact, this isn't a valid workaround as just using the delete button on images causes its own problem. The image file gets deleted from the file system but the page markdown still lists the (now broken) image file.

@mmorkt
Copy link

mmorkt commented Oct 18, 2018

Got the same problem:
My blueprint has multiple dropzones included in a list. If I ‘unset’ an image all images disappears in the frontmatter, the only image thats left is the one from the first dropzone that image is now on every dropzone.

@w00fz
Copy link
Member

w00fz commented Nov 12, 2018

The unset shouldn't really be showing on single file uploads. It's supposed to be available only when multiple is enabled.
In single file mode you are supposed to delete and reupload the image.

@hdwebpros
Copy link
Contributor

hdwebpros commented Nov 27, 2018

@w00fz there is still an issue. I had a client come across this.

If you utilize a list, and have a field within your list as a file, even if you set multple to false it still shows the unset icon. That is fine by me, because my client didn't want to delete the image across the entire site, just unset it and replace it.

If you unset a file (or image) it will set every single file within that list as one. I tested local and confirmed this bug.

@FalkoJoseph
Copy link

I'm running into the same issue. A client of mine tried to remove an image and had all images being replaced with the same image as being mentioned above.

@hdwebpros
Copy link
Contributor

Found out that the JS was reaching all hidden fields. I submitted (sloppily) a PR that I've tested locally and it works. But... of course I can't seem to submit the PR correctly.

There are other minor bugs that I've found, but this PR should at least fix this major issue.

PR: #1559
Commit: 54d800a

@w00fz w00fz closed this as completed Dec 14, 2018
@w00fz
Copy link
Member

w00fz commented Dec 14, 2018

If you guys need to test you need to use the beta, that’s where all the changes and fixes are going right now, especially for the media portion.

@hdwebpros
Copy link
Contributor

The fixes in 1.9 Beta don't address this issue. I submitted another PR #1561

For some reason, I can't seem to commit a clean PR, it always wants to include my upgrade to 1.9 in my fork for some reason. I'm sorry about that.

@mmorkt
Copy link

mmorkt commented Mar 5, 2019

It’s not fixed yet. Please remove the button or fix the bug. It has coursed some headaches

@hdwebpros
Copy link
Contributor

I tried a PR that fixes this. However, I am not good at pull requests.

I'll try again hopefully soon. The problem is obvious and I know the fix.

@hdwebpros
Copy link
Contributor

I'm not sure why this is still closed. I tried submitting a PR #1559 but that got shut down.

If I could do a PR off of the main branch, I could solve this quickly.

4 or so people on here see this as a problem, and 3 of our clients already do. It's easy to replicate:

  • Make a page use a blueprint with a type of list, each entry with an image field
  • Add different images to all
  • Unset an image and replace, click save

I have the fix too! I just can't seem to do a PR off of a branch that isn't the main branch

@rhukster
Copy link
Member

What are you using to create the PR? First fork the 1.6 branch, then when you create the PR, it should let you pick 1.6 as the base by default in github.

@hdwebpros
Copy link
Contributor

How do you recommend I fork a specific branch? When I click on the fork button, I get https://github.com/hdwebpros/grav-plugin-admin

That's been my issue the whole time. I can't seem to fork off of a specific branch.

I asked around, and people mentioned stuff about using git to checkout upstream and remote or something like that but it wasn't as simple as just fork, clone, do edits, do PR

@rhukster
Copy link
Member

Follow these steps:

  1. Fork the getgrav/grav-plugin-admin repo to your hdwebpros account (you've done this already but maybe delete yours and start over).
  2. Clone your hdwebpros/grav-plugin-admin repo on your local machine
  3. Switch your local repo to 1.9 branch
  4. Make your edits and test with your clone
  5. Commit your changes to your repo (still in 1.9 branch)
  6. In Github you should get a prompt to create a Pull Requests. It should see the changes in 1.9 branch but it also provides a UI to change:

Image 2019-03-27 at 10 14 00 AM

@hdwebpros
Copy link
Contributor

Maybe deleting my fork will help here. Thanks for the tips! I'll give it a whirl

@hdwebpros
Copy link
Contributor

hdwebpros commented Mar 28, 2019

Alright.

After much finaggling, I got my local 1.6 beta with my forked admin plugin switched to branch 1.9 to work.

I flat out don't even see an unset button. Anywhere. It seems to be in the code, but not in the repo's compiled code. Am I missing something?

Furthermore, when trying to run npm run dev from the themes/grav folder, (after I successfully ran npm install) I get an error.

Output filename not configured.

There is no output filename configured in the Webpack conf file. I'm stuck here, can't compile the JS to test my fixes.

@juliendude
Copy link

juliendude commented Apr 7, 2019

A quick fix or work around, for now, that seems to work for me is to set the limit to a high number.

@mmorkt
Copy link

mmorkt commented Apr 16, 2019

Didn't work for me.
Got multiple dropzones (like the one below) in a list, each time I use "unset" all images are being replaced by the first image from the first dropzone.

.files:
  type: file  
  label: Files goes here
  style: vertical
  multiple: true
  limit: 9999999999999
  destination: 'user/pages/assets/dokument'
  accept:
    - .pdf

@julienbenisty how did you use Limit. Do you have a simple YAML file to show?

@hdwebpros
Copy link
Contributor

I would submit to fix if I could get this running locally.

The problem was that the unset JavaScript targets every single drop zone you have.

I wish I could be more of an assistance to you.

@rotanadan
Copy link
Contributor

Pull request to fix this at #1670

@hdwebpros
Copy link
Contributor

Here's an update. The Pull Request has a review request: #1670 It looks like @w00fz will be taking a look at it. If anyone has any other input they'd like to share, or if you can test the PR as well, it would help the Grav team out. I've tested it and it works fine btw

@w00fz
Copy link
Member

w00fz commented May 8, 2019

This is now solved, I merged @rotanadan PR.

Thanks all for the help and patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests