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

feat: Revamps editor for message and article #6145

Merged
merged 21 commits into from
Jan 16, 2023
Merged

Conversation

nithindavid
Copy link
Contributor

@nithindavid nithindavid commented Dec 29, 2022

Description

This PR introduces separate editors for the message reply box and article editor in the help center. With support for keyboard rules to trigger formatting marks like bold, italics, code, lists etc.

Will update list of new changes introduced as a doc, meanwhile please see the comment on test cases.

Introduces new icons on the menu
image

Improves UI for canned response selector
image

Loom video for message editor: https://www.loom.com/share/10f36094f4b14bf0b48d8c6f9ca17a3d
Loom video for article editor: https://www.loom.com/share/6dea1e175a384bc39e1e69b9e0a92645

Notes while testing

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality not to work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@pr-triage pr-triage bot added the PR: draft The pull request is not ready to be reviewed label Dec 29, 2022
@netlify
Copy link

netlify bot commented Dec 29, 2022

Deploy Preview for chatwoot-storybook ready!

Name Link
🔨 Latest commit 74e3446
🔍 Latest deploy log https://app.netlify.com/sites/chatwoot-storybook/deploys/63c537fb8b03380008eea763
😎 Deploy Preview https://deploy-preview-6145--chatwoot-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@nithindavid nithindavid marked this pull request as ready for review January 6, 2023 16:11
@pr-triage pr-triage bot added PR: unreviewed This pull request is yet to be reviewed. and removed PR: draft The pull request is not ready to be reviewed labels Jan 6, 2023
@nithindavid
Copy link
Contributor Author

nithindavid commented Jan 6, 2023

Cases tested on Message Editor

Menu

  • All menu items toggles the correct marks [Bold, italics, Code, Link]
  • Menu shows active state correctly
  • Undo redo works correctly
  • Can Add ul, ol through menu
  • Ul, Ol icons gets hidden on first item of a menu
  • Can nest lists inside lists from second item
  • Can add a link on highlighted text

Keyboard Shortcuts

  • Wrapping inside **stars** transforms text into bold
  • Wrapping inside *stars* transforms text into italics
  • Wrapping inside `stars` transforms text into code
  • * and space to start unordered list
  • 1. and space to start ordered list

Existing behavior

  • Enter to send and Cmd+enter works correctly to send message
  • Shift+enter does soft break from current line
  • Can paste content into editor
  • Can insert Emoji
  • Typing \ shows canned response menu
  • Typing text after \ filters content in canned response menu
  • Typing @ shows canned response menu
  • Can insert canned response through mouse click
  • Can insert canned response through keyboard navigation
  • Can insert mention through mouse click
  • Can insert mention through keyboard navigation
  • Can attach files

@nithindavid
Copy link
Contributor Author

I was able to build the storybook locally, Will fix the build issue here.

image

@pranavrajs
Copy link
Member

@nithindavid Nice work here! This is a real improvement in the editor. The help center editor feels awesome.

Some minor suggestions.

  1. Can we avoid the placeholder here?

Screenshot 2023-01-06 at 1 18 12 PM

  1. Can we reduce the spacing? It seems a bit much.

Screenshot 2023-01-06 at 1 20 47 PM

@pranavrajs
Copy link
Member

Also, there is an issue with the code editor. The start of each line is not aligned properly.
Screenshot 2023-01-06 at 1 21 29 PM

@nithindavid
Copy link
Contributor Author

@nithindavid Nice work here! This is a real improvement in the editor. The help center editor feels awesome.

Some minor suggestions.

  1. Can we avoid the placeholder here?
Screenshot 2023-01-06 at 1 18 12 PM
  1. Can we reduce the spacing? It seems a bit much.
Screenshot 2023-01-06 at 1 20 47 PM

@pranavrajs Is spacing issue between the menu and title or the typography?

@nithindavid
Copy link
Contributor Author

@pranavrajs Thanks for the feedback :) I have fixed the comments.

Copy link
Member

@iamsivin iamsivin left a comment

Choose a reason for hiding this comment

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

@nithindavid Tested this PR locally, and it's working as expected. Found an issue,

  1. If enter to send is enabled, we cannot break the line to the new one.
    https://www.loom.com/share/30eb69e31992438998586bc011b0b5d9

  2. Feels not aligned properly if the first item is not active, same in help center

image

Copy link
Member

@scmmishra scmmishra left a comment

Choose a reason for hiding this comment

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

Screenshot 2023-01-11 at 5 03 01 PM

The changes mostly look good to me, found this small UI bug. There's no bottom padding in the canned responses list, is it intentional?

Copy link
Member

@scmmishra scmmishra left a comment

Choose a reason for hiding this comment

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

Some nitpicks

In the article view for my screensize, the text looks way too big Screenshot 2023-01-11 at 5 14 16 PM
The title area seems to be resizable, can we disable that?
Screen.Recording.2023-01-11.at.5.16.25.PM.mov
In the editor, you can see two scrollbars for some reason, can we fix that?
Screen.Recording.2023-01-11.at.5.20.13.PM.mov

@nithindavid
Copy link
Contributor Author

@scmmishra @iamsivin Thanks for the review. I'll fix them soon.

There's no bottom padding in the canned responses list; is it intentional?
I removed the bottom padding so when the list overflows, it'll look like there are items to scroll. I'll add padding for the last item to fix this.

@nithindavid
Copy link
Contributor Author

nithindavid commented Jan 11, 2023

In the article view for my screensize, the text looks way too big

@scmmishra This was done with initial feedback on the old editor to match the typography on the article view page. eg: https://chatwoot.help/hc/handbook/en/engineering/15. The font size is 18px here. Please let me know what you think.

cc: @pranavrajs

@pranavrajs
Copy link
Member

This was done with initial feedback on the old editor to match the typography on the article view page. eg: https://chatwoot.help/hc/handbook/en/engineering/15. The font size is 18px here. Please let me know what you think.

@scmmishra @nithindavid Let us try to match the experience end user is getting so that it doesn't like an entirely different content when the user publishes it.

@nithindavid
Copy link
Contributor Author

@iamsivin @scmmishra I have made the fixes. Can you let me know if this is good to merge?

Copy link
Member

@iamsivin iamsivin left a comment

Choose a reason for hiding this comment

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

I tested this PR locally and it's working as expected 🔥

@pr-triage pr-triage bot added PR: partially-approved Not all reviewers have approved the PR and removed PR: unreviewed This pull request is yet to be reviewed. labels Jan 13, 2023
Copy link
Member

@scmmishra scmmishra left a comment

Choose a reason for hiding this comment

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

LGTM!!!

@pr-triage pr-triage bot added PR: unreviewed This pull request is yet to be reviewed. and removed PR: partially-approved Not all reviewers have approved the PR labels Jan 16, 2023
@pr-triage pr-triage bot added PR: partially-approved Not all reviewers have approved the PR and removed PR: unreviewed This pull request is yet to be reviewed. labels Jan 16, 2023
@pr-triage pr-triage bot added PR: unreviewed This pull request is yet to be reviewed. and removed PR: partially-approved Not all reviewers have approved the PR labels Jan 16, 2023
@pranavrajs pranavrajs merged commit e707778 into develop Jan 16, 2023
@pranavrajs pranavrajs deleted the feat/revamp-editor branch January 16, 2023 17:08
@pr-triage pr-triage bot added PR: merged The pull request is merged to another branch and removed PR: unreviewed This pull request is yet to be reviewed. labels Jan 16, 2023
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR: merged The pull request is merged to another branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants