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

fix: spacing after list in rich text editor [WPB-15685] #18720

Merged
merged 6 commits into from
Feb 10, 2025

Conversation

waseemansar
Copy link
Contributor

@waseemansar waseemansar commented Feb 9, 2025

TaskWPB-15685 [Web] Fix spacing after list in the rich text editor

Description

This PR addresses two issues:

  1. Correct handling of empty lines in markdown rendering.
  2. TypeScript error related to the domain property in the MentionText interface.

Changes:

  1. Adjusted paragraph_open and paragraph_close rules in messageRenderer.ts to handle empty lines correctly only for lists.
  2. Ensured proper addition of <br> tags for empty lines between paragraphs.
  3. Updated the MentionText interface to allow domain to be of type string | null | undefined.

Screenshots/Screencast (for UI changes)

Before

issue

After

fix

Checklist

  • mentions the JIRA issue in the PR name (Ex. [WPB-XXXX])
  • PR has been self reviewed by the author;
  • Hard-to-understand areas of the code have been commented;
  • If it is a core feature, unit tests have been added;

@codecov-commenter
Copy link

codecov-commenter commented Feb 9, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 45.73%. Comparing base (7d4676a) to head (ef1e0e2).
Report is 9 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev   #18720   +/-   ##
=======================================
  Coverage   45.72%   45.73%           
=======================================
  Files         944      944           
  Lines       27872    27876    +4     
  Branches     6289     6292    +3     
=======================================
+ Hits        12745    12748    +3     
- Misses      13536    13537    +1     
  Partials     1591     1591           

@waseemansar waseemansar force-pushed the fix/list-spacing-WPB-15685 branch from 6004ce8 to a393333 Compare February 9, 2025 15:39
@@ -27,7 +27,7 @@ import {replaceInRange} from './StringUtil';
import type {MentionEntity} from '../message/MentionEntity';

interface MentionText {
domain: string | null;
domain: string | null | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
domain: string | null | undefined;
domain?: string | null;

Copy link
Contributor Author

@waseemansar waseemansar Feb 10, 2025

Choose a reason for hiding this comment

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

There was a typescript error for below code.

mentionTexts[mentionKey] = {
        domain: mention.domain,

mention.domain can be undefined.

@@ -99,6 +99,15 @@ markdownit.renderer.rules.paragraph_open = (tokens, idx) => {
.find(({map}) => map?.length);
const previousPosition = previousWithMap ? (previousWithMap.map || [0, 0])[1] - 1 : 0;
const count = position - previousPosition;

// Check if the previous token was a list (either bullet or ordered)
Copy link
Contributor

Choose a reason for hiding this comment

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

imo this comment is redundant, the code describes itself enough

@waseemansar waseemansar merged commit 1d68e99 into dev Feb 10, 2025
15 checks passed
@waseemansar waseemansar deleted the fix/list-spacing-WPB-15685 branch February 10, 2025 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants