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

Bullet point in list with multiple images not round trippable #555

Open
janaki-r-bhagwath opened this issue Oct 1, 2024 · 3 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@janaki-r-bhagwath
Copy link
Collaborator

janaki-r-bhagwath commented Oct 1, 2024

Description
If a single bullet point in a list has 2 images - it gets converted to 2 bullet points

To Reproduce
Source HTML

<ul>
    <li>
        <picture>
           ...
        </picture>
        <picture>
            ...
        </picture>
    </li>
    <li>
        <picture>
            ...
        </picture>
        <picture>
            ...
        </picture>
    </li>
</ul>

Expected MD

- ![][image0]![][image1]
- ![][image2]![][image3]

Generated MD

- ![][image0]

  ![][image1]
- ![][image2]

  ![][image3]

The new line feeds in this md get converted to bullet points when md2docx is used.

Expected behavior
The generated md should be

- ![][image0]![][image1]
- ![][image2]![][image3]

Screenshots

Version:
run: $ hlx --version

Additional context
Add any other context about the problem here.

@janaki-r-bhagwath janaki-r-bhagwath added the bug Something isn't working label Oct 1, 2024
@buuhuu
Copy link
Collaborator

buuhuu commented Oct 2, 2024

This seems to be the same as #529 It is not related. The list/listItem is not spread in this case.

The additional <p> is added by https://github.com/syntax-tree/hast-util-to-mdast/blob/main/lib/util/wrap.js#L51-L67 independently of the number of <picture> tags.

I added a test case here: 25b19ac

@tripodsan
Copy link
Contributor

tripodsan commented Oct 2, 2024

The additional <p> is added

but that's ok. maybe should just wrap the <img><img> with a <p>, before hast-to-mdast:

<li>
  <p><img><img></p>
</li>

@buuhuu
Copy link
Collaborator

buuhuu commented Oct 2, 2024

It is ok, and yes we could do that. However, the behaviour is different to doc-based. There the <picture>s are not wrapped in <p>s.

Maybe we should not try to have the exact same output generated for html2md compared to docx2md & co.? That is the same discussion as in #529

Ruchika4 added a commit to adobecom/unity that referenced this issue Oct 28, 2024
…n unity block (#134)

- Remove thumbnail from authoring for change background in unity block
- This update is done to fix an issue with html localization flow
adobe/helix-html2md#555

Resolves: [MWPW-159445](https://jira.corp.adobe.com/browse/MWPW-159445)

Test URLs:

Before:
https://main--cc--adobecom.hlx.page/drafts/ruchika/remove-background1?unitylibs=stage
After:
https://main--cc--adobecom.hlx.page/drafts/ruchika/remove-background1?unitylibs=loc
sanjayms01 pushed a commit to adobecom/unity that referenced this issue Oct 28, 2024
…n unity block (#134)

- Remove thumbnail from authoring for change background in unity block
- This update is done to fix an issue with html localization flow
adobe/helix-html2md#555

Resolves: [MWPW-159445](https://jira.corp.adobe.com/browse/MWPW-159445)

Test URLs:

Before:
https://main--cc--adobecom.hlx.page/drafts/ruchika/remove-background1?unitylibs=stage
After:
https://main--cc--adobecom.hlx.page/drafts/ruchika/remove-background1?unitylibs=loc

errors.
Ruchika4 added a commit to adobecom/unity that referenced this issue Oct 29, 2024
#143)

Stage to main sync PR (Stage PR (#134))

- Remove thumbnail from authoring for change background in unity block
- This update is done to fix an issue with html localization flow
adobe/helix-html2md#555

Resolves: [MWPW-159445](https://jira.corp.adobe.com/browse/MWPW-159445)

Test URLs:

Before:

https://main--cc--adobecom.hlx.page/drafts/ruchika/remove-background1?unitylibs=main
After:

https://main--cc--adobecom.hlx.page/drafts/ruchika/remove-background1?unitylibs=stage
Ruchika4 added a commit to adobecom/unity that referenced this issue Nov 4, 2024
…unity block for V2 (#146)

- Remove thumbnail from authoring for change background in unity block
for V2
- This update is done to fix an issue with html localization flow
adobe/helix-html2md#555

Resolves: [MWPW-159445](https://jira.corp.adobe.com/browse/MWPW-159445)

Test URLs:

Before:
https://stage--unity--adobecom.hlx.page/drafts/ruchika/remove-background
After:

https://thumbnail-v2--unity--adobecom.hlx.page/drafts/ruchika/remove-background
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants