-
Notifications
You must be signed in to change notification settings - Fork 32
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
RFC 60: Draftail Usage for General Text Entry #60
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to what the RFC covers at the moment, I’d like to see our plan for any areas where a <input type="text">
and <textarea>
differ with Draftail. Thinking of two in particular:
max_length
If set, this will be used server-side for validation / storage, but browsers will also automatically truncate any text entry that goes beyond the maxlength
. I presume we’d need to re-implement this – and give special consideration to Unicode gotchas like surrogate pairs so the counting is identical client-side and server-side. MDN says maxlength
counts "UTF-16 code units".
This will be a bit annoying to have to reinvent but an upside is that we could also use this as an opportunity to introduce a character count for all fields in a way that’s consistent.
Accessibility
Visually I’m sure we can style Draftail to look almost identical to <input type="text">
and <textarea>
. I wonder how big of a difference it would be for screen reader users though – I think it needs looking into. For the RFC, at least stating we intend for the fields to be announced similarly even if semantically they might be different. Worth considering how to convey a field is single-line vs. multiline for example? Checking what browsers might announce when maxlength
is reached, if anything?
Other considerations
Things I wondered about and wasn’t sure whether they’d need looking into:
- Support for browser extensions such as Grammarly or vim shortcuts that might work very well with native elements but not with our custom widget.
- Reviewing the list of know Draftail issues for things that are relevant for plain text, and we might want to spend more time assessing if we used Draft.js for a lot more fields
- How this RFC overlaps with Outline RFC 46: Single paragraph rich text fields/blocks #46
Adding further notes from my research: Accessibility
max_length
|
For people following this – the RFC is now tagged as In particular, we’re considering:
|
Put on the core team chat also. My general concern is that we may be causing some known bugs in Draftail to now be propagated to all text fields. As long as there is an easy opt in. (update. Originally wrote opt out) springload/draftail#138 I have not validated these myself - maybe they are fixed now. |
We will also want to ensure that we support all native Dom events like blur/focus/click/change etc. Personally I'm less convinced this is a good direction - we are heading towards native date picker for the reason that it's simpler, browser support is good and works without maintenance. We may be creating something that adds a lot of overhead to simple fields in terms of complexity, reinventing the wheel and supporting all browsers and accessibility targets well. As such I think this should be opt in only and be implemented like a custom widget. |
@lb- |
Hey. Sorry what I meant was that we are potentially replacing plain text fields with a full JS driven react component right? My concern is that we are making something the default that is more complex for simple text fields. |
Draft.js has confirmed that it is no longer maintained. The project will no longer receive updates and the GitHub repo will be archived in October of this year (October 2022). I think we need to consider this if we are to adopt our wrapper around Draft.js in even more places in Wagtail. This is frustrating I know but I personally do not think we can proceed with this RFC unless we have a more solid underpinning for a large change across basically any text field in Wagtail. Maybe we can find a different way to solve for comments, feedback, and other features we want to implement around common fields. facebookarchive/draft-js#3091 (comment) |
This is unfortunate news and I think we should probably adjust our implementation of this RFC to be editor-agnostic where possible. Perhaps change the RFC itself to be completely editor-agnostic, and just say Wagtail’s rich text editor should be usable for fields saved as plain-text, and detail what that would look like. Aside from this I don’t see how Facebook’s announcement makes Draft.js any less suitable of an underpinning, for this kind of feature, within Wagtail, in the mid term. We’ve been using Draft.js v0.10.5 since 4 years ago when we set it up without ever doing any upgrades, so although some of the known issues are annoying, they’ve never been annoying enough for us to really dig into. If we did have concerns about those bugs, we could still look into upgrading Draft.js (we have about 2.5 years of releases to catch up on), or fixing them outside the framework. Re your previous comment @lb-,
If we want to support the features envisioned in the RFC, I don’t see any other way than to increase the complexity of plain-text fields. And regardless of which editor we use, I believe a JS-driven |
@jacobtoppm just coming back to all "Final Comment Period" RFCs – do you have thoughts about my suggestion above, to make this RFC editor-agnostic? Stipulate Wagtail’s rich text pipeline should support "plain text" fields, from rendering to editor widgets. As we consider the future of rich text in Wagtail, I think it’d be better for us to get this kind of fundamental design choice decided on before we consider our options (investing in Draftail / Draft.js more, or fully focus on alternatives). |
@thibaudcolas I see what you mean. Should we move away from Draftail, we should have some other "rich" (for annotation/analysis type features) wrapper around plain text, which should probably be the new editor, yep, so I think it makes sense for at least some part of our rich text pipeline to be usable for plain text fields too. Most of the differences to this RFC would be on the FE/widget details side - essentially on the Django side we only need a widget that takes a string to render its initial data, and returns a string when converting back from POST data. The However, currently Draftail is where inline commenting is implemented, and that's a pretty big motivation for this. Performance testing was also done with Draftail only, so I don't think stripping the RFC down to make it completely generic adds that much? Maybe just adding another section/a prelude to say "this should apply whatever editor we choose; now here are the Draftail considerations" |
@jacobtoppm sounds reasonable to me! I’ve made edits towards this direction, let me know what you think. |
Co-authored-by: Thibaud Colas <thibaudcolas@gmail.com>
Co-authored-by: Thibaud Colas <thibaudcolas@gmail.com>
Co-authored-by: Thibaud Colas <thibaudcolas@gmail.com>
Thanks @thibaudcolas, I agree with the suggestions - added them in. |
…aaand merged. There’s no plans for anyone to work on this in the short term, so still plenty of time for discussion if we have second thoughts. #8247 could be a good place to comment on, though I suspect we’re more likely to go ahead with #8249 (support for single-line rich text fields) than straight to plain-text "rich text" fields. |
Rendered