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

Fixed draft js formatting issues #1690

Merged
merged 15 commits into from
Oct 8, 2021
Merged

Fixed draft js formatting issues #1690

merged 15 commits into from
Oct 8, 2021

Conversation

mdshamoon
Copy link
Member

@mdshamoon mdshamoon commented Oct 6, 2021

Summary

Added decorator for bold and removed markdown formatting internally.
Formattting done now will now be similar to whatsApp

@cypress
Copy link

cypress bot commented Oct 6, 2021



Test summary

180 0 0 0Flakiness 3


Run details

Project Glific
Status Passed
Commit bf19da3 ℹ️
Started Oct 8, 2021 10:20 AM
Ended Oct 8, 2021 10:42 AM
Duration 21:40 💡
OS Linux Ubuntu - 20.04
Browser Electron 91

View run in Cypress Dashboard ➡️


Flakiness

flow/Flow.spec.ts Flakiness
1 Flow > should display flow keyword below the flow name
staffmanagement/StaffManagement.spec.ts Flakiness
1 Staff Management > should have require field
template/HSMTemplate.spec.ts Flakiness
1 HSM Template > should show typed sample message in simulator

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@codecov
Copy link

codecov bot commented Oct 7, 2021

Codecov Report

Merging #1690 (02e3445) into master (3a8acd8) will increase coverage by 0.20%.
The diff coverage is 40.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1690      +/-   ##
==========================================
+ Coverage   77.13%   77.34%   +0.20%     
==========================================
  Files         206      206              
  Lines        7007     7001       -6     
  Branches     1500     1507       +7     
==========================================
+ Hits         5405     5415      +10     
+ Misses       1134     1110      -24     
- Partials      468      476       +8     
Impacted Files Coverage Δ
src/components/UI/Form/Input/Input.tsx 95.83% <ø> (ø)
src/containers/Form/FormLayout.tsx 62.38% <0.00%> (ø)
src/mocks/InteractiveMessage.tsx 100.00% <ø> (ø)
...mponents/UI/Form/WhatsAppEditor/WhatsAppEditor.tsx 47.82% <28.57%> (-13.29%) ⬇️
...ntainers/InteractiveMessage/InteractiveMessage.tsx 64.02% <33.33%> (+4.29%) ⬆️
src/containers/Template/Form/Template.tsx 36.48% <37.50%> (+0.27%) ⬆️
src/components/UI/Form/EmojiInput/EmojiInput.tsx 56.94% <40.00%> (-9.73%) ⬇️
...rs/InteractiveMessage/InteractiveMessage.helper.ts 75.00% <50.00%> (ø)
...ntainers/Chat/ChatMessages/ChatInput/ChatInput.tsx 79.61% <60.00%> (ø)
src/common/RichEditor.tsx 81.35% <66.66%> (+7.54%) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a8acd8...02e3445. Read the comment docs.

@mdshamoon mdshamoon requested a review from kurund October 7, 2021 12:30
@mdshamoon mdshamoon self-assigned this Oct 7, 2021
@kurund
Copy link
Contributor

kurund commented Oct 7, 2021

Here is an odd behavior:

Before fix:
Screenshot 2021-10-07 at 10 25 47 PM

After fix:
Screenshot 2021-10-07 at 10 26 20 PM

Ideally, the text should not be bold and if we are making it bold then * should not be shown

@kurund
Copy link
Contributor

kurund commented Oct 7, 2021

Select the text and use the keyboard shortcut command + b, you will see the below error after few seconds.

Screenshot 2021-10-07 at 10 34 37 PM

Copy link
Contributor

@kurund kurund left a comment

Choose a reason for hiding this comment

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

let's fix the errors before merging this.

@kurund
Copy link
Contributor

kurund commented Oct 7, 2021

type 'test'

then add * around it. text is converted to bold and * are still shown.

Screenshot 2021-10-07 at 10 41 15 PM

It is behaving correctly in simulator

Screenshot 2021-10-07 at 10 42 15 PM

After message is sent it is shown as bold

@mdshamoon
Copy link
Member Author

Here is an odd behavior:

Before fix: Screenshot 2021-10-07 at 10 25 47 PM

After fix: Screenshot 2021-10-07 at 10 26 20 PM

Ideally, the text should not be bold and if we are making it bold then * should not be shown

@kurund I was keeping it like how WhatsApp does it but yeah I think this can confuse people. Let me remove it.

@mdshamoon mdshamoon requested a review from kurund October 8, 2021 06:13
Copy link
Contributor

@kurund kurund left a comment

Choose a reason for hiding this comment

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

@mdshamoon

Few issuees:
Screenshot 2021-10-08 at 8 32 09 AM

  • message text is bold but it should not
  • Preview reduces number of *

@mdshamoon mdshamoon requested a review from kurund October 8, 2021 10:07
Copy link
Contributor

@kurund kurund left a comment

Choose a reason for hiding this comment

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

@mdshamoon
PR looks good!

Working as expected and all the reported issues are fixed.

Good to merge once CI passes

@mdshamoon mdshamoon merged commit cd83adb into master Oct 8, 2021
@mdshamoon mdshamoon deleted the fix/draft-js-formatting branch October 8, 2021 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Asterisk keep increasing and changing their place with each save action performed.
2 participants