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

Only remove attachment upload button after trix has initialized #3464

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iainbeeston
Copy link
Contributor

Description

I'm finding that when attachments are disabled the trix field controller is trying to remove the attachments button before trix has initialized and before it has created the toolbar. This crashes the controller inside the connect method and so none of the other event listeners are set up and the attach files button isn't removed.

NOTE: I haven't been able to test this myself, the code is my best guess at how to fix the issue.

Fixes # (issue)

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Screenshots & recording

Screenshot 2024-11-26 at 14 41 39

Manual review steps

  1. Find a resource with a trix field
  2. Make sure it shows an attachment button
  3. Disable attachments
  4. Make sure the attachment button is now hidden

Manual reviewer: please leave a comment with output from the test if that's the case.

I'm finding that when attachments are disabled the trix field controller is trying to remove the attachments button before trix has created the toolbar. This crashes the controller inside the connect method and so none of the other event listeners are set up.
Copy link

codeclimate bot commented Nov 26, 2024

Code Climate has analyzed commit 74c0b69 and detected 0 issues on this pull request.

View more on Code Climate.

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

Successfully merging this pull request may close these issues.

1 participant