Skip to content
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.

[Rust] fix multiple lines in markdown mode #907

Merged
merged 6 commits into from
Dec 14, 2023

Conversation

Velin92
Copy link
Member

@Velin92 Velin92 commented Dec 13, 2023

Markdown mode was not able to parse newlines, and when the number of consecutive newlines was more than 1 the model generation panicked, resulting in an empty DOM when sending the message.
This makes markdown mode more stable, by allowing newlines to be parsed correctly.

This PR also removes the shiftEnter KeyCommand from iOS since we never actually use it/override in any project, and it just creates the issue of bypassing the correct shift+enter behaviour in markdown mode, which is the behaviour for the software return keyboard on iOS.
Is probably best to completely remove it until we find a better architecture to handle the key commands.

@codecov-commenter
Copy link

codecov-commenter commented Dec 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (efe567d) 87.49% compared to head (aa4a7a9) 89.49%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #907      +/-   ##
============================================
+ Coverage     87.49%   89.49%   +2.00%     
============================================
  Files           166      118      -48     
  Lines         18993    16912    -2081     
  Branches       1030      616     -414     
============================================
- Hits          16617    15136    -1481     
+ Misses         2074     1752     -322     
+ Partials        302       24     -278     
Flag Coverage Δ
uitests 78.89% <ø> (+7.62%) ⬆️
uitests-android ?
uitests-ios 78.89% <ø> (ø)
unittests 88.17% <100.00%> (+2.61%) ⬆️
unittests-android ?
unittests-ios 75.80% <ø> (+0.25%) ⬆️
unittests-rust 89.43% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@jmartinesp jmartinesp left a comment

Choose a reason for hiding this comment

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

The Rust changes LGTM, but maybe we could add some comments to explain why they're needed?

…nto mauroromito/fix_markdown_multiple_lines

# Conflicts:
#	platforms/ios/example/Wysiwyg/Views/ContentView.swift
#	platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygComposerView.swift
#	platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygComposerView/WysiwygTextView.swift
#	platforms/ios/lib/WysiwygComposer/Sources/WysiwygComposer/Components/WysiwygKeyCommand.swift
@Velin92 Velin92 requested a review from jmartinesp December 14, 2023 09:25
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@Velin92 Velin92 removed the iOS label Dec 14, 2023
@Velin92 Velin92 merged commit d43db0a into main Dec 14, 2023
7 checks passed
@Velin92 Velin92 deleted the mauroromito/fix_markdown_multiple_lines branch December 14, 2023 10:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants