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

List image append #2346

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

SabrePenguin
Copy link

Adds a break tag between and that prevents the tag from being forced into the last list item when generating html from markdown.

This prevents an issue which is visible from #2155

Signed-off-by: SabrePenguin <lknofczynski@gmail.com>
Signed-off-by: SabrePenguin <lknofczynski@gmail.com>
@Trial97
Copy link
Member

Trial97 commented Apr 25, 2024

Did you look at:#2341?
This should fix the issue regarding image width that you mentioned

@SabrePenguin
Copy link
Author

I did. This fixes a different issue, which I mentioned on Discord two days ago. #2341 fixes image width. This fixes images being forcefully appended to lists when they shouldn't.

@Trial97
Copy link
Member

Trial97 commented Apr 25, 2024

Can you still reproduce the mentioned issue with my PR? If yes can you also do a screenshot and explain what is wrong?

@SabrePenguin
Copy link
Author

SabrePenguin commented Apr 25, 2024

No, your pr fixes the width attribute when I pull your changes. Here's what I'm focused on. On Modrinth, it's a new line and separate from the list.
image

@Trial97
Copy link
Member

Trial97 commented Apr 25, 2024

This does look like an issue but I'm sure that is not with prism but cmark
And I think it can be fixed in a cleaner there where the html is generated.

@SabrePenguin
Copy link
Author

It's not with cmark. I did testing and the generated html displays properly with both firefox and chromium. I don't know how Prism handles end of lists, but some pages have this bug, while others don't.

@TheKodeToad
Copy link
Member

So this is likely an issue with Qt?

@SabrePenguin
Copy link
Author

It's likely that Qt is at fault. But I'm not used to using Qt, so I can't really test effectively with it yet.

@Trial97
Copy link
Member

Trial97 commented Apr 26, 2024

From what I can tell this is indeed a QT bug.

launcher/Markdown.cpp Outdated Show resolved Hide resolved
@Trial97
Copy link
Member

Trial97 commented Apr 26, 2024

Also because this is a QT bug we should consider extracting this code in a separate function, and instead of having it be called here to be called each time we call setHtml wrapping the string in the said function.
And because I could not find the a QT bug that describes this I created one:https://bugreports.qt.io/browse/QTBUG-124793

Co-authored-by: Alexandru Ionut Tripon <alexandru.tripon97@gmail.com>
Signed-off-by: SabrePenguin <147069705+SabrePenguin@users.noreply.github.com>
launcher/Markdown.cpp Outdated Show resolved Hide resolved
Co-authored-by: Alexandru Ionut Tripon <alexandru.tripon97@gmail.com>
Signed-off-by: SabrePenguin <147069705+SabrePenguin@users.noreply.github.com>
launcher/Markdown.cpp Outdated Show resolved Hide resolved
Co-authored-by: TheKodeToad <TheKodeToad@proton.me>
Signed-off-by: SabrePenguin <147069705+SabrePenguin@users.noreply.github.com>
@Trial97 Trial97 added bug Something isn't working simple change changelog:fixed A PR that appears under "Fixed" in the changelog backport release-8.x Backport PR automatically labels Apr 27, 2024
@Trial97 Trial97 added this to the 8.4 milestone Apr 27, 2024
Signed-off-by: SabrePenguin <lknofczynski@gmail.com>
Signed-off-by: SabrePenguin <lknofczynski@gmail.com>
Signed-off-by: SabrePenguin <lknofczynski@gmail.com>
Signed-off-by: SabrePenguin <lknofczynski@gmail.com>
Signed-off-by: SabrePenguin <lknofczynski@gmail.com>
@SabrePenguin SabrePenguin requested a review from Trial97 May 1, 2024 04:42
launcher/StringUtils.cpp Outdated Show resolved Hide resolved
launcher/StringUtils.cpp Outdated Show resolved Hide resolved
SabrePenguin and others added 2 commits May 1, 2024 12:49
Co-authored-by: TheKodeToad <TheKodeToad@proton.me>
Signed-off-by: SabrePenguin <147069705+SabrePenguin@users.noreply.github.com>
Signed-off-by: SabrePenguin <lknofczynski@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-8.x Backport PR automatically bug Something isn't working changelog:fixed A PR that appears under "Fixed" in the changelog simple change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants