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(markdown): use new markdown crate #1336

Closed
wants to merge 44 commits into from
Closed

Conversation

sdwoodbury
Copy link
Contributor

@sdwoodbury sdwoodbury commented Oct 16, 2023

What this PR does 📖

  • uses the markdowns crate, produced by Satellite.im.
  • Transforms ascii emojis to unicode ex: :O -> 😮
  • detects messages that consist of only emojis and whitespace and enlarges them
  • makes markdown support configurable in settings
  • doesn't support lists yet
  • adds css for markdown relates classes, fixing the markdown bugs present in dev. (the markdown crate isn't technically needed to do this but the two have been tested together).

Which issue(s) this PR fixes 🔨

Special notes for reviewers 🗒️

Additional comments 🎤

@github-actions github-actions bot added Don't merge yet DO NOT MERGE Missing dev review Still needs to be reviewed by a dev labels Oct 16, 2023
@phillsatellite
Copy link
Contributor

phillsatellite commented Oct 17, 2023

Hey Stu I found a couple things while testing with the most latest commit (Verified that the commit Im on has the reply fix that you just recently added in) These are able to be replicated on all OS's

  • It doesnt seem like all the markdowns show up in replies, Bold appeared fine for me but all the other markdowns wouldnt appear
Screen.Recording.2023-10-17.at.12.43.59.PM.mov
  • Not all markdowns are appearing in the sidebar, bold appears fine but none of the others will appear
Screenshot 2023-10-17 at 12 45 23 PM
  • Markdowns arent appearing in Pinned messages
Screenshot 2023-10-17 at 12 44 43 PM
  • I also couldn't get underline markdowns to work

  • I used the code markdown and put "cargo run --release" in there and you can see in screenshot it just says "run --release"

Screenshot 2023-10-17 at 1 19 37 PM

@phillsatellite phillsatellite added Changes requested When other dev or QA request a change and removed Ready for testing Ready for QA to test labels Oct 17, 2023
@sdwoodbury
Copy link
Contributor Author

@phillsatellite underline isn't supported.

@stavares843 stavares843 added the linter failing Cargo Workflow (linter) failed on this PR label Oct 17, 2023
@stavares843
Copy link
Member

Captura de ecrã 2023-10-18, às 01 21 40

is there specific ascii that are allowed? some didnt converted into emoji, example the : P one

@luisecm
Copy link
Contributor

luisecm commented Oct 18, 2023

Looks like the existing markdown to send code in a message is not working with this PR and thats why the chats tests are failing

@sdwoodbury sdwoodbury marked this pull request as draft October 18, 2023 15:24
@sdwoodbury
Copy link
Contributor Author

Captura de ecrã 2023-10-18, às 01 21 40

is there specific ascii that are allowed? some didnt converted into emoji, example the : P one

yes. i think i'll move that code into uplink instead of the crate. that way it will be more clear to us. as for the above list, it's too much. don't care about most of those. :P will be added.

@sdwoodbury sdwoodbury marked this pull request as ready for review October 18, 2023 15:38
@sdwoodbury sdwoodbury marked this pull request as draft October 18, 2023 15:39
@sdwoodbury
Copy link
Contributor Author

I found that code blocks don't display html tags - probably because messages are displayed using dangerous_html. needs some more thought.

@sdwoodbury sdwoodbury closed this Oct 19, 2023
@github-actions github-actions bot removed Changes requested When other dev or QA request a change Missing dev review Still needs to be reviewed by a dev Don't merge yet DO NOT MERGE Failed Automated Test This PR is failing Luis's Appium test and needs revised linter failing Cargo Workflow (linter) failed on this PR labels Oct 19, 2023
@stavares843 stavares843 deleted the fix/markdown-again branch October 19, 2023 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: PR Open
Development

Successfully merging this pull request may close these issues.

bug(markdown): markdown never worked
5 participants