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

Update attachment button unnecessarily displayed #3873

Closed
Ben-MyBB opened this issue Jan 5, 2020 · 7 comments · Fixed by #4113
Closed

Update attachment button unnecessarily displayed #3873

Ben-MyBB opened this issue Jan 5, 2020 · 7 comments · Fixed by #4113
Labels
b:1.8 Branch: 1.8.x s:resolved Status: Resolved. Solution implemented or scheduled t:bug Type: Bug. An issue causing error / flaw / malfunction
Milestone

Comments

@Ben-MyBB
Copy link
Member

Ben-MyBB commented Jan 5, 2020

Completely confused about "Update Attachment" button. The thing its doing currently is nothing but what "Add Attachment" does. So why the button is there? Can anyone guide me in which step this buttons action is unique?

This button should be removed or if really intended to update the attachment - it should be multiple, per already attached file list.
Point me out if I am missing something.

And now the bug:

We have switched to AJAX file remove. Well.
Update attachment appears only if atleast one file is already attached. But AJAX delete does not respect it.
Remove all the already attached files - "Update Attachment" button is still there. Refresh the page - its gone.

Original thread: Update attachment

@Ben-MyBB Ben-MyBB added b:1.8 Branch: 1.8.x s:confirmed Status: Confirmed. Retested and found the issue exists t:bug Type: Bug. An issue causing error / flaw / malfunction labels Jan 5, 2020
@Eldenroot
Copy link
Contributor

@lairdshaw what do you think, you have done a lot of work with attachments lately :)

@lairdshaw
Copy link
Contributor

lairdshaw commented May 21, 2020

@Eldenroot Agreed that this button is useful, as explained by doylecc in the original thread (post number 3). Also agreed that it should not be visible when there are no longer any attachments. It should be pretty simple to remove/hide it in Javascript if/when "Remove" is clicked (and confirmed) for the last attachment.

@rajat315315
Copy link
Contributor

I think it does the same work as deleting an attachment with name "xyz" and then re-adding some attachment with the same name.
So, in this case, it seems redundant.

@lairdshaw
Copy link
Contributor

I think it does the same work as deleting an attachment with name "xyz" and then re-adding some attachment with the same name.

It does one other thing which makes it non-redundant and worth keeping: it maintains the same attachment ID.

@rajat315315
Copy link
Contributor

@lairdshaw for the user updating his attachment, does it even matter?

@lairdshaw
Copy link
Contributor

@rajat315315 Yes, it sometimes does: when the attachment is included already in one or more posts using [attachment=the_attachment_id].

@Sama34
Copy link
Contributor

Sama34 commented Jul 11, 2020

@lairdshaw for the user updating his attachment, does it even matter?

If we remove we would be removing a feature that is useful for some people and might be used by somebody.

@Ben-MyBB Ben-MyBB added s:feedback Status: Feedback. Further information/input needed and removed s:confirmed Status: Confirmed. Retested and found the issue exists labels Jul 15, 2020
@euantorano euantorano added s:resolved Status: Resolved. Solution implemented or scheduled and removed s:feedback Status: Feedback. Further information/input needed labels Nov 6, 2020
@euantorano euantorano added this to the 1.8.25 milestone Nov 6, 2020
@dvz dvz changed the title Update attachment Update attachment button unnecessarily displayed Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:1.8 Branch: 1.8.x s:resolved Status: Resolved. Solution implemented or scheduled t:bug Type: Bug. An issue causing error / flaw / malfunction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants