Skip to content

Make WYSIWYG editor reusable#6293

Merged
PeerRich merged 41 commits intomainfrom
fix/reusable-wysiwyg
Jan 9, 2023
Merged

Make WYSIWYG editor reusable#6293
PeerRich merged 41 commits intomainfrom
fix/reusable-wysiwyg

Conversation

@CarinaWolli
Copy link
Copy Markdown
Member

@CarinaWolli CarinaWolli commented Jan 5, 2023

What does this PR do?

  • Makes text editor that is used for custom workflow templates reusable

    • By default the toolbar includes block type, bold, italic and link. The new prop excludedToolbarItems: string[] allows to exclude the ones not needed.
  • Also makes the 'Add variable' dropdown reusable by introducing a new prop variables: string[] (if variables is undefined or empty the 'Add variable' button is not showing).

    • renamed all translation keys for dynamic text variables form _workflow to _variable
  • Block type dropdown had a broken design. We now use our own Dropdown component

Fixes #6267

Loom Video 1. (shows that everything is working with new dropdown and the possibility to remove 'Add Variable' button): https://www.loom.com/share/a8c90b69264d45a1950bd14ca2637e2c

Loom Video 2 (shows how to remove items from the toolbar): https://www.loom.com/share/ed37bdfbef864bacb5024ace67337b29

Environment: Staging(main branch)

How should this be tested?

  • Test all functionalities of the editor and see if everything is still working as expected

@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 5, 2023

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

Name Status Preview Updated
cal ✅ Ready (Inspect) Visit Preview Jan 9, 2023 at 2:14PM (UTC)

@github-actions github-actions Bot added automerge ♻️ autoupdate tells kodiak to keep this branch up-to-date labels Jan 5, 2023
Comment thread packages/ui/components/editor/Editor.tsx
? language.t(variable.toLowerCase().concat("_workflow")).replace(/ /g, "_").toLocaleUpperCase()
: originalVariables.includes(variable.toLowerCase().concat("_name_workflow")) //for the old variables names (ORGANIZER_NAME, ATTENDEE_NAME)
? language.t(variable.toLowerCase().concat("_name_workflow")).replace(/ /g, "_").toLocaleUpperCase()
const regex = new RegExp(`{${variable}}`, "g"); // .replaceAll is not available here for some reason
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This (+ changes in line 31) fixed a bug in translating some variables in some languages

setShowBlockOptionsDropDown: Dispatch<SetStateAction<boolean>>;
};

function BlockOptionsDropdownList({
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removing the BlockOptionsDropdownList as we are now using our Dropdown component

/>,
document.body
)}
<Dropdown>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Using our Dropdown component here

@github-actions github-actions Bot added automerge ♻️ autoupdate tells kodiak to keep this branch up-to-date labels Jan 7, 2023
@PeerRich
Copy link
Copy Markdown
Member

PeerRich commented Jan 9, 2023

testing right now.

i know the following changes are not part of this PR but could be addressed here:

not centered:
CleanShot 2023-01-09 at 14 55 20@2x

active and hover should not have border radius:
image

icon not centered, also not sure if we want border radius here:
CleanShot 2023-01-09 at 14 56 34@2x

@kodiakhq kodiakhq Bot removed the automerge label Jan 9, 2023
@kodiakhq
Copy link
Copy Markdown
Contributor

kodiakhq Bot commented Jan 9, 2023

This PR currently has a merge conflict. Please resolve this and then re-add the automerge label.

@PeerRich
Copy link
Copy Markdown
Member

PeerRich commented Jan 9, 2023

i dont think we will bring back test action due to the potential for abuse so we should remove it

CleanShot 2023-01-09 at 14 57 59@2x

unless we rate limit or similar very soon. but this should not be visible + disabled for too long

Copy link
Copy Markdown
Member

@PeerRich PeerRich left a comment

Choose a reason for hiding this comment

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

tested and works! added comments unrelated to this PR which can be addressed here or in a follow up PR

@PeerRich PeerRich merged commit a0135cd into main Jan 9, 2023
@PeerRich PeerRich deleted the fix/reusable-wysiwyg branch January 9, 2023 14:15
@leog
Copy link
Copy Markdown
Contributor

leog commented Jan 9, 2023

@CarinaWolli Looks good, agree with Peer's feedback on UI. The only NIT I would mention is maintaining two different lists of variables, one with "_variable" at the end and the other without that suffix, maybe one list could live in a constants file if makes sense, so it can be used in different places.

@PeerRich PeerRich added the core area: core, team members only label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

♻️ autoupdate tells kodiak to keep this branch up-to-date core area: core, team members only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CAL-676] Make WYSIWYG Editor reusable

4 participants