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

[WIP] add required attribute to trix-editor #1144

Closed
wants to merge 1 commit into from

Conversation

metamoni
Copy link

@metamoni metamoni commented Mar 25, 2024

Closes #1143

Context

<trix-editor> currently accepts autofocus and placeholder attributes, but not a required attribute. This PR aims to add that in.

This PR is still in progress. I'm stuck because even though I've set required to true on the inputElement, the validation is not triggered. I'm not sure what's missing here, so any help would be appreciated. The inputElement currently has a readonly attribute; I tried removing that, but it made no difference.

To make manual testing easier I've updated assets/index.html with an example of a plain textarea that's required. The validation gets triggered on this field alright, but not on the trix-editor.

Changes

  • added a makeRequired function to trix_editor_element.js
  • set required to true on the inputElement, when the required attribute is present on trix-editor

@metamoni metamoni marked this pull request as ready for review March 25, 2024 09:54
@KonnorRogers
Copy link

KonnorRogers commented Mar 25, 2024

@metamoni The problem is that validations will get skipped if the input element isn't visible.

So any <input type="hidden"> will get skipped.

There's a couple options:

1.) Wait for @seanpdoyle 's PR for formAssociation and write a custom validator. (not sure which one #1128 #1121 )
2.) The shorter term fix for this PR:

Modify the <input> element generated by Trix to look like this:

- <input type="hidden">
+ <input
  style="
    clip-path: inset(50%);
    height: 1px;
    overflow: hidden;
    position: absolute;
    white-space: nowrap; 
    width: 1px;
  "
>

Now the problem with the above is you probably want a relatively positioned container and then probably want to set it to:

bottom: 0;
left: 0;

So that it doesn't get weird alignment like this:

Screenshot 2024-03-25 at 12 38 25 PM

But yea, TLDR: can't use type="hidden", either need a "visually-hidden" <input> or use FormAssociated.

The problem IIRC is the <input> is generated server side, so when Trix connects you'd need to modify it.

@metamoni
Copy link
Author

metamoni commented Mar 25, 2024

@KonnorRogers thanks for explaining. Is it not the textarea that we're looking to modify here then? This is the element I see from element.inputElement, when developing locally. I don't understand why it's the hidden input that needs to have the required attribute, would you be able to briefly clarify that?

Happy to wait if there's a solution being built 👍

@metamoni metamoni closed this Mar 25, 2024
@KonnorRogers
Copy link

@metamoni The problem is <trix-editor> isn't what gets submitted when you submit the form. its the hidden input. The <form> has no clue what <trix-editor> is. It may look like a <textarea>, but if you inspect the DOM, it's just a contenteditable element.

By default forms only respect required attributes on: <button>, <input>, <select>, etc without using ElementInternals on the custom element.

Trix has been working around this limitation for a long time by "Mirroring" its value to the hidden input element. So the only way to get validation working today without changes is to modify the hidden input because that's what the <form> is looking at. I don't know if that made any sense 😥

@metamoni
Copy link
Author

@KonnorRogers it did! This was my first time using trix, so I wasn't sure how it worked behind the scenes, but I definitely understand more about it now. Thanks for taking the time to explain it!

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.

Add the option to make the textarea required
2 participants