-
Notifications
You must be signed in to change notification settings - Fork 8
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
Nimble Rich Text Editor and Viewer Spec Document #1289
Conversation
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address suggestions to important sections
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor-viewer/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor/specs/README.md
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor/specs/README.md
Outdated
Show resolved
Hide resolved
Since this is your first PR, I'll just mention: now that I'll the Nimble reviewers have approved you're welcome to complete it yourself once all the status checks are passing. Looks like the build just finished and the last comment thread is almost resolved, so you should be just about good to go! π |
@vivinkrishna-ni As Jesse mentioned once the status checks are complete and you have the owners you need you can merge the PR. Is there anything y'all are waiting on? |
@rajsite , as @RickA-NI is working on the visual designs for the comments feature, we have planned to analyze that and update any content if required. Will it be good to submit these changes now and initiate another PR in the future if any change needs to be done in the document? Please advice. |
Yes, that would be a good way to do it, especially since #1314 is currently waiting on this. |
@suseendran-ni you stated you will merge the PR but then closed it. If you want to merge it you need to enable auto-merge / click merge, not close. Did you mean to close the PR or merge the PR? |
Thanks for pointing this out @rajsite . I have merged the branch now. |
β¦1416) # Pull Request ## π€¨ Rationale This PR introduces the initial implementation of the editor component by building on top of the `Tiptap` editor. The component is developed to align with the design for the comments feature and matches the current visual design linked below. [Visual design](https://www.figma.com/file/PO9mFOu5BCl8aJvFchEeuN/Nimble_Components?type=design&node-id=2482-82389&mode=design&t=Kl5FdGYvvpvs9BY8-0) [Comments feature mockup](https://www.figma.com/file/Q5SU1OwrnD08keon3zObRX/SystemLink?type=design&node-id=8773-161649&mode=design&t=ZKp2UlDUmvMa56p9-0) AzDo Feature: https://dev.azure.com/ni/DevCentral/_backlogs/backlog/ASW%20SystemLink%20LIMS/Features/?workitem=2350963 Issue: #1288 Other functionalities mentioned in the spec document for rich text editor will be implemented in subsequent PRs. ## π©βπ» Implementation * Installed latest version of [@tiptap/core](https://www.npmjs.com/package/@tiptap/core) and other extensions for the supported nodes and marks. * Updated the spec for installing the actually required packages instead of `@tiptap/starter-kit`. * Initialized the editor with the extensions by importing from their respective packages. * Added `nimble-toolbar` for the footer section. * Added `nimble-toggle-button` for the supported formatting options such as bold, italics, numbered list, and bulleted list and added functionalities to it. `nimble-toggle-button` uses `click` and `keydown` events to toggle the state instead of `change` event. The rationale for adopting this approach is elaborated as follows: * For more details on the issue, see #1289 (comment). * It is due to the fact that the `change` event toggles upon clicking, whereas the intended behavior is for the state to change only based on the node that currently holds the cursor focus. * Since the underlying component of the toggle button functions as a switch, we must update the checked state in the template and also modify the handling of change and click events when employing this event approach. * On the other hand, in the `click` event approach, we merely reference it and update the state whenever needed. * We prototyped both the `change` event and `click` event approach and decided to use the `click` event as it is simpler to implement. For more info, refer to the discussion [here](https://dev.azure.com/ni/DevCentral/_backlogs/backlog/ASW%20SystemLink%20Platform/Initiatives/?workitem=2419268). * Added `footer-actions` slot element. * Style the component in a way to match the nimble theme. ![image](https://github.com/ni/nimble/assets/123377523/98c0552c-0e5f-4eca-979c-d5d592772280) ## π§ͺ Testing * Added unit tests and visual sizing tests for the component. * Manually tested and verified the functionality of the supported features. ## β Checklist <!--- Review the list and put an x in the boxes that apply or ~~strike through~~ around items that don't (along with an explanation). --> - [x] I have updated the project documentation to reflect my changes or determined no changes are needed. ---------
Pull Request
π€¨ Rationale
Creating spec for
nimble-rich-text-editor
andnimble-rich-text-viewer
components.Issue: #1288
π©βπ» Implementation
This PR contains the nimble spec document for the new nimble component addition. The spec document consists of a design discussion for the
nimble-rich-text-editor
andnimble-rich-text-viewer
components and explains the external libraries used for the development of a rich text editor.π§ͺ Testing
N/A
β Checklist
N/A