feat(rich-text-editor) - single click actions #633
Conversation
|
Demo will be published at https://apps.inindca.com/common-ui-docs/genesys-webcomponents/feature/COMUI-2963_liteDOM |
github-actions
bot
requested review from
katie-bobbe-genesys and
MattCheely
There was a problem hiding this comment.
Will be tokenising this and the other files shortly.
gavin-everett-genesys
changed the title
|
I am going to update this PR to include all the actions that just require a single click since its pretty straight forward to implement. For actions that require dropdowns / popovers I will create seperate. |
...onents/beta/gux-rich-text-editor/gux-rich-text-editor-action/gux-rich-text-editor-action.tsx
Show resolved
Hide resolved
|
The icon doesn't seem vertically centred, probably default line height. |
|
Ordered lists, unordered lists, Block Quotes throwing errors: |
|
Should delete clear the content? |
Ignore that, seems to be working & I can't reproduce the issue. |
| <h1>gux-rich-text-editor</h1> | ||
| <gux-rich-text-editor-beta> | ||
| <gux-rich-text-editor-action-group slot="typographical-emphasis"> | ||
| <gux-rich-text-editor-action action="bold"></gux-rich-text-editor-action> |
There was a problem hiding this comment.
should this be gux-rich-text-editor-button?
There was a problem hiding this comment.
In the designs they are referenced as actions. Also wanted to keep it consistent with the name we have for actions in the table-toolbar as its similar.
| } | ||
|
|
||
| .tiptap { | ||
| font-size: 14px; |
There was a problem hiding this comment.
What is this styling is every app going to need something like this?
There was a problem hiding this comment.
This is tiptap styling but you can also have no styling at all and just have the default editor styles. I have configured it to have the same stylings as the figma designs. Its optional really but might be handy to have it as a template I guess. I could add a comment over it maybe.
There was a problem hiding this comment.
Should we be including it in a light dom css file that is shipped with the components?
There was a problem hiding this comment.
Yep this makes sense to me 👍
...onents/beta/gux-rich-text-editor/gux-rich-text-editor-action/gux-rich-text-editor-action.tsx
Outdated
Show resolved
Hide resolved
Looking for a workaround atm, theres a few open issues on github surrounding it |
addition of rich-text-editor ✅ Closes: COMUI-2963
gavin-everett-genesys
force-pushed
the
feature/COMUI-2963_liteDOM
branch
from
697ffca
to
b3fff53
Compare
fix divider gaps ✅ Closes: COMUI-2963
update width of rte ✅ Closes: COMUI-2963
join import statements ✅ Closes: COMUI-2963
There seems to a broken CDN import from https://esm.sh. I changed the imports to use jsdelivr instead and it works fine. |
move tiptap styles to light scss file ✅ Closes: COMUI-2963
gavin-everett-genesys
requested review from
daragh-king-genesys and
conor-darcy
| > | ||
| <div class="gux-rich-text-editor-toolbar-container"> | ||
| <div class="gux-typographical-emphasis-container"> | ||
| <slot name="typographical-emphasis"></slot> |
There was a problem hiding this comment.
Should we conditionally render these slots? Or better still, the wrappers.
There was a problem hiding this comment.
Yeah that makes sense to me 👍
| <slot name="typographical-emphasis"></slot> | ||
| </div> | ||
| <div class="gux-text-styling-container"> | ||
| <slot name="text-styling"></slot> |
There was a problem hiding this comment.
I'm not sure we need the named slots, would one single slot not work where you just slot in multiple gux-rich-text-editor-action-group's?
This would be more flexible allowing devs to construct the toolbar as they see fit...may UX want this restricted?
There was a problem hiding this comment.
In the designs in figma it states that each group should have its own slot. I am open to change but thinking how the responsive behaviour will work also. At the moment I can use the wrapper classes potentially for hiding and showing those action groups whereas if were just using a single slot I am not sure how that will work. Open to suggestions
| return ( | ||
| <div class="gux-action-group-divider"> | ||
| <div class="gux-divider"></div> | ||
| </div> |
There was a problem hiding this comment.
You can probably do this with one div.
...gux-rich-text-editor/gux-rich-text-editor-action-group/gux-rich-text-editor-action-group.tsx
Show resolved
Hide resolved
| type="button" | ||
| disabled={this.disabled} | ||
| class={{ 'gux-is-active': this.isActive }} | ||
| aria-pressed={this.isActive.toString()} |
There was a problem hiding this comment.
Do we need an aria-label or some way to describe the button as the icon is decorative?
I know the label is in the tooltip but to got to tab past the button into the tooltip to get the label.
There was a problem hiding this comment.
Think the use of screenreader-text property on the the icon will suffice 🤔
| .prosemirror pre { | ||
| white-space: pre-wrap; | ||
| } | ||
|
|
There was a problem hiding this comment.
Is there a better class to hang the styles off?
If you use another library you won't get .prosemirror or .tiptap.
There was a problem hiding this comment.
Good spot, I am now just applying an editor-styles class to the editor on load. I assume other editors might have the same functionality where they can add a class.
address PR comments ✅ Closes: COMUI-2963
address PR comments ✅ Closes: COMUI-2963
This change includes the following,
Next steps :
I am currently in the process of tokenising this.