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

[WC-2835]: RTE implementation of Quill-Table-Better #1506

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

samuelreichert
Copy link
Contributor

Pull request type


Description

@samuelreichert samuelreichert force-pushed the feat/WC-2835-RTE-table branch 3 times, most recently from a42830a to 3a4ce62 Compare March 24, 2025 08:28
@samuelreichert samuelreichert force-pushed the feat/WC-2835-RTE-table branch from 3a4ce62 to 8b801ae Compare March 27, 2025 15:40
@samuelreichert samuelreichert marked this pull request as ready for review March 28, 2025 08:40
@samuelreichert samuelreichert requested a review from a team as a code owner March 28, 2025 08:40
leftIcon.setAttribute("src", left);
dropDown.appendChild(leftIcon);
if (isDropDown) {
const rightIcon = document.createElement("img");
Copy link
Collaborator

@gjulivan gjulivan Mar 31, 2025

Choose a reason for hiding this comment

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

seems like it is possible to not use svg and use rich text font icons here.
i have notify Ana to update RTE font icons.
and we can reduce bundle size

}

createMenu(left: string, right: string, isDropDown: boolean) {
const container = document.createElement("div");
Copy link
Collaborator

@gjulivan gjulivan Mar 31, 2025

Choose a reason for hiding this comment

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

update all document into this.quill.root.ownerDocument or this.root.ownerDocument
this is safer if rich text being put into iframe.

const EXTRA: MenusDefaults = {
copy: {
content: useLanguage("copyTable"),
icon: copyIcon,
Copy link
Collaborator

Choose a reason for hiding this comment

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

if using RTE font icons, this probably changed into iconClassName

}

createDragTable(table: Element) {
const dragTable = document.createElement("div");
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here, change document into ownerDocument

: false
table: false,
"table-better": {
language: "en_US",
Copy link
Collaborator

Choose a reason for hiding this comment

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

we are using hardcoded language here.
let's confirm with @takumagott first.
otherwise, we can use ownerDocument.documentElement.lang to check for browser's language.
but note that we don't support all languages.

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

Successfully merging this pull request may close these issues.

3 participants