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

#7630: Fix DraftJS support on empty fields #7644

Merged
merged 5 commits into from
Feb 17, 2024

Conversation

fregante
Copy link
Contributor

@fregante fregante commented Feb 17, 2024

What does this PR do?

Review tips

The actual bug fix can be seen in e436878 (#7644). The rest of the changes are to extract the utility functions and test some

Demo

Tested on https://draftjs.org

Some fields are available on https://ghosttext.fregante.com/test/ (but not DraftJS at this point)

Screen.Recording.5.mov

Checklist

  • Add tests
  • New files added to src/tsconfig.strictNullChecks.json (if possible)
  • Designate a primary reviewer: @BLoe

@fregante fregante added the bug Something isn't working label Feb 17, 2024
Copy link

codecov bot commented Feb 17, 2024

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (2fb7ff7) 72.94% compared to head (7007a05) 72.94%.

Files Patch % Lines
src/utils/domFieldUtils.ts 75.00% 14 Missing ⚠️
src/bricks/effects/forms.ts 87.50% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7644   +/-   ##
=======================================
  Coverage   72.94%   72.94%           
=======================================
  Files        1230     1231    +1     
  Lines       38309    38324   +15     
  Branches     7188     7192    +4     
=======================================
+ Hits        27943    27956   +13     
- Misses      10366    10368    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

/**
* Set the value of an input, doing the right thing for check boxes, etc.
*/
async function setValue({
async function findAndSetValue({
Copy link
Contributor Author

@fregante fregante Feb 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the generic "value-setting" logic out of this function and file. They could be replaced in the future.

Setting values into custom editors is complex. We might eventually want to see how Grammarly does it. However they have issues with a lot of sites too.

Some editors expose a DOM-accessible API, but are increasingly more "private" and difficult/impossible to access (like Monaco editor). Some examples are in

https://github.com/fregante/GhostText/blob/main/source/advanced-editors-messenger.js

Copy link

No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack.

Do not edit this comment manually.

@grahamlangford grahamlangford merged commit b52f229 into main Feb 17, 2024
20 checks passed
@grahamlangford grahamlangford deleted the F/bug/support-draft-js-better branch February 17, 2024 14:38
@grahamlangford grahamlangford added this to the 1.8.9 milestone Feb 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set Input Value brick clears whole page if DraftJS editor is empty
3 participants