pdfme / pdfme

A TypeScript based PDF generator library, made with React.
https://pdfme.com
MIT License
2.17k stars 192 forks source link

contentEditable firefox fix #444

Closed valushagrinchik closed 2 months ago

valushagrinchik commented 2 months ago

As contentEditable = 'plaintext-only' doesn't work in firefox, this workaround fixes it and keeps 'plaintext-only' functionality (avoiding of styles be pasted)

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments | Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **pdfme-playground** | ⬜️ Ignored ([Inspect](https://vercel.com/labelmake/pdfme-playground/5yYVy9X9f8PGC8bBosVyvG1suke7)) | [Visit Preview](https://pdfme-playground-git-fork-valushagrinchik-cont-8e6818-labelmake.vercel.app) | | Mar 14, 2024 9:29am | | **pdfme-playground-v4** | ⬜️ Ignored ([Inspect](https://vercel.com/labelmake/pdfme-playground-v4/2Qff3heEMAWMudQASbfKR6bkBBKc)) | [Visit Preview](https://pdfme-playground-v4-git-fork-valushagrinchik-c-fff3b8-labelmake.vercel.app) | | Mar 14, 2024 9:29am |
hand-dot commented 2 months ago

Hey @valushagrinchik

Thank you creating PR!!

@peteward

it's also possible to format text in contentEditable=true elements in other ways, such as Cmd/Ctrl + B/I/U (amongst others).

Indeed, I hadn't considered this case.

I'd like to hear your opinion. After your advice, @valushagrinchik modified the process to change it using try-catch, but I wonder if it would be better to switch the process with a function like the one below?

const isFirefox = () => navigator.userAgent.toLowerCase().indexOf("firefox") > -1;

I think there's no problem with the rest of it. I'd be very happy either way, as it will improve the critical issue of being unable to edit text in Firefox.

peteward commented 2 months ago

Being explicit about the browser seems like a good idea. We know that it's only FF out of current browsers that has the issue so it would be safe. It's also clearer in code why there are different approaches.

However, there's more to consider. When you type in contenteditable=true you will automatically get <div> and <br> tags inserted when you type. We need to find a way to handle this:

Screenshot 2024-03-14 at 05 47 25

peteward commented 2 months ago

This fiddle appears to show a solution for handling the keypresses and replacing with newline characters.

peteward commented 2 months ago

@valushagrinchik would you mind updating to include the check for Firefox explicitly and handling newlines without creating HTML elements?

valushagrinchik commented 2 months ago

This fiddle appears to show a solution for handling the keypresses and replacing with newline characters.

@peteward It looks like the incorrect link was attached, so that I couldn't check what you suggested Code that I added also prevent creating HTML elements but I noticed that this line textBlock.innerText = value; transforms \n to <br/>. Could it be a problem?

peteward commented 2 months ago

Hi @valushagrinchik ,

apologies, here is the correct link: https://jsfiddle.net/1te5hwv0/

I think when you paste content in it might be OK, but when you press ENTER while typing it creates <div> elements for each line.

The screenshot above was from your branch which I tried locally.

peteward commented 2 months ago

Hi @valushagrinchik,

Are you working on this at the moment? If not I can try to finish it today.

Thanks.

peteward commented 2 months ago

Hi @valushagrinchik,

Good job! This works well for me when testing locally. The dev tools confusingly show
elements but these don't appear to exist when you export the template or render to PDF, so guessing this is just a quirk with devtools and how it's rendering the new lines.

@hand-dot can you review please?

hand-dot commented 2 months ago

Thank you @peteward I'll test it tomorrow morning and then release it!

hand-dot commented 2 months ago

Released! https://github.com/pdfme/pdfme/releases/tag/3.4.2

Thank you @valushagrinchik and @peteward !