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

HIGH: [Polish] Add support for image-only (no alt text) inline image Markdown #38952

Closed
quinthar opened this issue Mar 25, 2024 · 9 comments
Closed
Assignees
Labels

Comments

@quinthar
Copy link
Contributor

Problem:

In HIGH: [Polish] Add support for inline image Markdown we added support for ![alt text](imageURL) syntax, which is great! However, that presumes you always have "alt text", which in my experience, is rare.

Solution:

Add support for !(imageURL), which doesn't use any alt text.

@quinthar quinthar converted this from a draft issue Mar 25, 2024
@quinthar
Copy link
Contributor Author

@kidroca can you comment here and/or self-assign? (I can't assign... for some dumb GH limitation I can't explain).

@kidroca
Copy link
Contributor

kidroca commented Mar 25, 2024

I'm not able to self-assign myself to this issue, but this comment should allow anyone with access to assign me

@kidroca
Copy link
Contributor

kidroca commented Apr 3, 2024

The pull request (PR) for expensify-common is ready for review:

However, the existing bug with live-markdown and MD image, gets in the way here as well. While trying to submit a PR for NewDot to allow QA testing of the Markdown (MD) image shorthand syntax, I encountered the same bug (detailed in #39275) using the latest main branch. This bug prevents the usage of images with shorthand syntax. Although I confirmed the feature works with an older commit, submitting a PR based on it isn't feasible because PRs have to be based on a recent version of main.

The tricky part is the resolution to the live markdown preview issue depends on a fix included in my expensify-common PR (the on at the top), as described here #39275 (comment).

Since testing this fix directly in NewDot is impossible ATM, I recommend integrating the expensify-common update into the live markdown preview PR. This combined approach appears to be the most effective solution:

@kidroca
Copy link
Contributor

kidroca commented Apr 12, 2024

@kidroca
Copy link
Contributor

kidroca commented Apr 18, 2024

One potential issue is that we usually use the NewDot PR to test changes, but this PR automatically closed when I synced it with main, as there were no remaining differences.

I've provided detailed testing instructions in the PR, which QA can use to validate the new feature.

@quinthar
Copy link
Contributor Author

quinthar commented May 4, 2024

Can we close this?

@kidroca
Copy link
Contributor

kidroca commented May 7, 2024

Can we close this?

Yes

@melvin-bot melvin-bot bot removed the Weekly KSv2 label May 30, 2024
Copy link

melvin-bot bot commented May 30, 2024

This issue has not been updated in over 15 days. @kidroca eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@melvin-bot melvin-bot bot added the Monthly KSv2 label May 30, 2024
Copy link

melvin-bot bot commented Jul 26, 2024

@kidroca, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

2 participants