Bug 1831315 - OLM: Tables with long strings don't render well in markdown viewier
Summary: OLM: Tables with long strings don't render well in markdown viewier
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: Management Console
Version: unspecified
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: ---
: 4.5.0
Assignee: Samuel Padgett
QA Contact: Yadan Pei
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2020-05-04 21:29 UTC by nathan.brophy
Modified: 2020-07-13 17:35 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Previously, markdown tables for OLM operators could render poorly when they had a lot of content. The web console has improved the display of these tables and added a horizontal scrollbar when necessary.
Clone Of:
Environment:
Last Closed: 2020-07-13 17:34:48 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
attachement 1 indented code blocks not properly rendered (1.14 MB, image/png)
2020-05-04 21:29 UTC, nathan.brophy
no flags Details
attachment 2 nested lists not rendered properlly (1.04 MB, image/png)
2020-05-04 21:33 UTC, nathan.brophy
no flags Details
attachment 3 table format not rendered correctly (1.04 MB, image/png)
2020-05-04 21:33 UTC, nathan.brophy
no flags Details
Verification screenshot (75.48 KB, image/png)
2020-05-12 06:40 UTC, XiaochuanWang
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Github openshift console pull 5314 0 None closed Bug 1831315: Improve formatting of markdown tables 2020-06-24 01:36:28 UTC
Red Hat Product Errata RHBA-2020:2409 0 None None None 2020-07-13 17:35:11 UTC

Description nathan.brophy 2020-05-04 21:29:44 UTC
Created attachment 1684980 [details]
attachement 1 indented code blocks not properly rendered

Description of problem:

The markdown renderer for the OCP administration console does not correctly render github flavored markdown.  If I have an OLM enabled operator that is visible in the operator catalog in the administration view, then the markdown in the spec.description field of the CSV for the OLM bundle gets rendered in the install tile.  The rendering of the markdown has some issues:

1. Nested lists need 4 spaces instead of 2 (github flavored mismatch) (attachment 2 [details])
2. Indented code blocks are not rendered properly (attachment 1 [details])
3. Tables that are too long get squished and text is vertically displayed, instead of a X overflow scroll being set (attachment 3 [details])


How reproducible:

This is reproducible consistently.   


Steps to Reproduce:

In the CSV for the OLM enabled operator add the following to the spec.description field:

 description: |
   - this list 
   - is flat
   - test 

    - this list 
      - is nested
        - test 

    - this is a list
      ```
      this is an indented code block
      ```

The markdown provided will not render correctly in the administration catalog for the operator, but will render correctly in github.

Actual results:

The markdown is not rendered properly, as shown in the example screen shots attached to the ticket.


Expected results:

I expect for the markdown to be rendered correctly, as I would expect to see it in github.  For issues 2 and 3 of the problem description, this is an expectation that markdown of that style will be correctly rendered.


Additional info:

Comment 1 nathan.brophy 2020-05-04 21:33:17 UTC
Created attachment 1684981 [details]
attachment 2 [details] nested lists not rendered properlly

Comment 2 nathan.brophy 2020-05-04 21:33:47 UTC
Created attachment 1684982 [details]
attachment 3 [details] table format not rendered correctly

Comment 3 nathan.brophy 2020-05-05 12:55:26 UTC
What markdown format is supported by the markdown renderer?  If 1 and 2 are supported in the markdown renderer today, then that is fine, I just need to know what format the renderer is expecting if not github flavored.  The table display appears to be a CSS issue however.

Comment 4 Samuel Padgett 2020-05-05 19:26:21 UTC
We use showdown:

https://github.com/showdownjs/showdown

It looks like we can set it to support GitHub flavor:

https://github.com/showdownjs/showdown#flavors

Can you share the CSV so we can test?

Comment 5 nathan.brophy 2020-05-05 19:30:32 UTC
Adding in a small example from the CSV:

spec.description: |
    README description goes here: 

    - this list 
    - is flat
    - test 

    - this list 
      - is nested
        - test 

    - this is a list
      ```
      this is an indented code block
      ```
    
    | example of a header with long name | this | table | has | a lot | of | columns |
    | ---------------------------------- | ---- | ----- | --- | ----- | -- | ------- |
    | data data data data data data data data data data data data data data data data data data ata data data data data data data data data data data data data data data data data data ata data data data data data data data data data data data data data data data data data | data | data | data | data | data | data |
    | data | data | data | data | data | data | data |
    | data | data | data | data | data | data | data |
    | data | data | data | data | data | data | data |


It seems like it is possible to render the above today (minus the table formatting) if write like the following:

- this list 
- is flat
- test 

- this list 
    - is nested
        - test 

- this is a list
    ```
    this is an indented code block
    ```

Here is a more complex and much larger README to use as a test case as well https://github.com/IBM/charts/blob/master/stable/ibm-websphere-liberty/README.md

Comment 6 Samuel Padgett 2020-05-06 13:39:10 UTC
The 4 space requirement is intentional behavior from showdown, which claims it's required by the spec and 2 spaces can cause problems.

https://github.com/showdownjs/showdown/commit/d51be6e
https://github.com/showdownjs/showdown/releases/tag/1.5.0

There is a `disableForced4SpacesIndentedSublists` option to let 2 spaces work, but even if set, 4 spaces are treated as one level of indentation. So

    - this list 
      - is nested
        - test 

renders as

    - this list 
      - is nested
      - test 

I'm hesitant to change that since it could change rendering of some other operator descriptions. I would recommend using 4 spaces which should work everywhere, including GitHub, and doesn't require an OpenShift version with this bug fix.

The table problem seems to be because we're setting `word-break: break-word`, which causes later columns to get squished. I plan to disable that and switch to horizontal scrolling tables (which is also what GitHub does).

Lowering severity since the list spacing is working as intended.

Comment 7 nathan.brophy 2020-05-06 13:57:43 UTC
I agree, can switch over to using 4 spaces everywhere for these README descriptions instead of 2.  Table formatting fix is not as high priority, and can come with a CSS change.  

Thank you for debugging and working through this with me.

Comment 10 XiaochuanWang 2020-05-12 06:40:00 UTC
Created attachment 1687532 [details]
Verification screenshot

Comment 11 XiaochuanWang 2020-05-12 06:41:29 UTC
Checked on     4.5.0-0.nightly-2020-05-11-202959
Attached the verification screenshot.
Is that the expected result?

Comment 12 Samuel Padgett 2020-05-12 12:37:42 UTC
(In reply to XiaochuanWang from comment #11)
> Checked on     4.5.0-0.nightly-2020-05-11-202959
> Attached the verification screenshot.
> Is that the expected result?

Yes, that is the expected result.

Comment 13 XiaochuanWang 2020-05-13 06:39:25 UTC
Thanks! This could be Verified as above result.

Comment 14 errata-xmlrpc 2020-07-13 17:34:48 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:2409


Note You need to log in before you can comment on or make changes to this bug.