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: add support for tasks and unordered list markdown #5057

Merged
merged 17 commits into from
Feb 7, 2024

Conversation

imf-ali
Copy link
Contributor

@imf-ali imf-ali commented May 5, 2023

Issue(s)

Tasks and unordered list does not appear in correct format as that in RC web app.

How to test or reproduce

  • Tasks should display in expected form
  • Unordered list should display in expected form

Screenshots

Previous behaviour
image

Improvement
image

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Fix #4927

@imf-ali
Copy link
Contributor Author

imf-ali commented May 5, 2023

Hey @diegolmello and @GleidsonDaniel , please have a look at the PR.

@imf-ali
Copy link
Contributor Author

imf-ali commented May 26, 2023

@GleidsonDaniel please have a look at it.

@reinaldonetof
Copy link
Contributor

A quick comment, instead add a new library to the repo, can you use it the same way as other places?

As you can see here:

<CustomIcon
testID={tshow ? 'send-to-channel-checked' : 'send-to-channel-unchecked'}
name={tshow ? 'checkbox-checked' : 'checkbox-unchecked'}
size={24}
color={themes[theme].auxiliaryText}
/>
we are using the custom icon for the checkbox

@GleidsonDaniel
Copy link
Contributor

GleidsonDaniel commented May 26, 2023

A quick comment, instead add a new library to the repo, can you use it the same way as other places?

As you can see here:

<CustomIcon
testID={tshow ? 'send-to-channel-checked' : 'send-to-channel-unchecked'}
name={tshow ? 'checkbox-checked' : 'checkbox-unchecked'}
size={24}
color={themes[theme].auxiliaryText}
/>

we are using the custom icon for the checkbox

@imf-ali i think this is the best approach.

Copy link
Contributor

@GleidsonDaniel GleidsonDaniel left a comment

Choose a reason for hiding this comment

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

Change your PR following the pattern commented by @reinaldonetof

@imf-ali
Copy link
Contributor Author

imf-ali commented May 27, 2023

Changes done. Please have a look @GleidsonDaniel @reinaldonetof .
Preview of the change
image

ios/Podfile.lock Outdated Show resolved Hide resolved
@imf-ali
Copy link
Contributor Author

imf-ali commented May 27, 2023

@GleidsonDaniel comment addressed. Please have a look.

ios/Podfile.lock Outdated Show resolved Hide resolved
app/containers/markdown/new/TaskList.tsx Outdated Show resolved Hide resolved
app/containers/markdown/new/TaskList.tsx Outdated Show resolved Hide resolved
@GleidsonDaniel
Copy link
Contributor

I saw that you sent me a private message asking for the review.
Just pay attention to one issue: Previously your PR was not reviewed because it didn't even pass the lint, so before requesting the PR check that all the steps in the checklist are correct.

@imf-ali
Copy link
Contributor Author

imf-ali commented Feb 7, 2024

I saw that you sent me a private message asking for the review. Just pay attention to one issue: Previously your PR was not reviewed because it didn't even pass the lint, so before requesting the PR check that all the steps in the checklist are correct.

Oh, sorry for the inconvenience. Will make sure next time, all the checks are successful. I have addressed the remaining comments. Kindly take a look once.

@GleidsonDaniel GleidsonDaniel merged commit c8e1f20 into RocketChat:develop Feb 7, 2024
3 of 8 checks passed
@GleidsonDaniel GleidsonDaniel changed the title Add support for tasks and unordered list markdown feat: add support for tasks and unordered list markdown Feb 7, 2024
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.

Add support for tasks and unordered list markdown
3 participants