Support the @Row directive

[dobromir-hristov]

Bug/issue #, if applicable: 97715742

Summary

This PR adds support for the new @Row directive in DocC.

Dependencies

DocC PR - swiftlang/swift-docc#378

Testing

Use the provided JSON file or insert the snippet below:

        {
          "type": "row",
          "numberOfColumns": 3,
          "columns": [
            {
              "size": 2,
              "content": [{
                "type": "text",
                "text": "Span 2"
              }]
            },
            {
              "content": [{
                "type": "text",
                "text": "Span 1 (auto)"
              }]
            }
          ]
        }

Steps:

1. Assert the grid works with specifying a column count and without.
2. Assert extra items overflow on a new line.
3. Assert on mobile, everything is equal width, vertical.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

• Added tests
• Ran npm test, and it succeeded
• Updated documentation if necessary

[dobromir-hristov]

One thing I am not sure I like is the naming. We already have GridRow and GridColumn... but these new ones use actual css grid... What is your opinions?

[mportiz08]

One thing I am not sure I like is the naming. We already have GridRow and GridColumn... but these new ones use actual css grid... What is your opinions?

I agree it's a little annoying but also not a huge deal, especially since these are located within the ContentNode folder along with the rest of the content components—should be clear from the surrounding context that these are specific to the Render Node directives.

[mportiz08]

After testing this with some slightly more realistic content (2 paragraphs), I think we'll also need to address some default gutter spacing between columns (both horizontal and vertical) so that the content has some room to breathe.

Here's screenshots of how it looks right now:

[dobromir-hristov]

After testing this with some slightly more realistic content (2 paragraphs), I think we'll also need to address some default gutter spacing between columns (both horizontal and vertical) so that the content has some room to breathe.

I was actually thinking about that the other day. We dont add any gutter to our GridRow and GridColumn, but those are building blocks, so it makes sense. Here however, I think as users wont have the possibility to add those, we should have nice defaults.

I will make it configurable via a prop, with some sensible defaults.

[dobromir-hristov]

Hey @mportiz08 I added the gap and all, but I am not sure if I like how the grid works when no column count is provided, aka auto mode.

Should we ensure we always fit up to a max of 12 columns, no matter how small they are, or do we add some sensible default?

This is something I saw thrown around on comment sections that is pretty clever. We can specify a max number of columns, the gap and the minimum size a column should be, and it should fit as many columns as it can, up to that --min-col-size.

12 in general is probably too much but yeah...

grid-template-columns: repeat(auto-fit, minmax(
     min(
       max(
         100% / var(--col-count) - var(--gap), 
         var(--min-col-size)
       ), 
       100%
     ), 
     1fr
));

[mportiz08]

Hey @mportiz08 I added the gap and all, but I am not sure if I like how the grid works when no column count is provided, aka auto mode.

The gap is looking good to me visually now, thanks!

Should we ensure we always fit up to a max of 12 columns, no matter how small they are, or do we add some sensible default?

I may be misunderstanding, but I don't think so. For example, I would expect the following to result in a grid with only 2 columns which are evenly spaced half/half, taking up the full width of the content area:

@Row {
  @Column {
    Foo
  }
  @Column {
    Bar
  }
}

I think the concept of an "N column" grid only applies when both a numberOfColumns argument is provided to the @Row directive and a child @Column directive contains a numeric size argument to enable authors with more control over how the columns are spaced.

For example, if someone wanted to create a row a very small first and last column and a big middle column:

@Row(numberOfColumns: 12) {
  @Column(size: 1) {
    Foo
  }
  @Column {
    Bar
  }
  @Column(size: 1) {
    Baz
  }
}

That's just based on the way I interpreted the proposal at Supporting more dynamic content in Swift-DocC reference documentation though, so @ethan-kusters please correct me if I misunderstand or I'm missing anything.

It's also just possible that you were asking if we should impose a maximum number of columns, but I'm also not sure about that. I don't think so, because maybe it would be reasonable to specify a 100 numCols with only 2 columns and one of them specifying size 5 to basically allow for a 5% width type of thing...maybe that's too much flexibility though and we want to enforce a 12 max—I'll need @ethan-kusters to give his thoughts there.

This is something I saw thrown around on comment sections that is pretty clever. We can specify a max number of columns, the gap and the minimum size a column should be, and it should fit as many columns as it can, up to that --min-col-size.

12 in general is probably too much but yeah...

grid-template-columns: repeat(auto-fit, minmax(
     min(
       max(
         100% / var(--col-count) - var(--gap), 
         var(--min-col-size)
       ), 
       100%
     ), 
     1fr
));

Going to take your word for it that the syntax is right there—the grid templating styles are so complex, haha. Sounds like an interesting idea as long as it plays nicely with what is allowed in the new markdown directive and its allowed argument values.

[mportiz08]

After testing this with some slightly more realistic content (2 paragraphs), I think we'll also need to address some default gutter spacing between columns (both horizontal and vertical) so that the content has some room to breathe.

I was actually thinking about that the other day. We dont add any gutter to our GridRow and GridColumn, but those are building blocks, so it makes sense. Here however, I think as users wont have the possibility to add those, we should have nice defaults.

I will make it configurable via a prop, with some sensible defaults.

How do you imagine the prop could be modified? Would that be intended as a possible future theme setting?

[dobromir-hristov]

How do you imagine the prop could be modified? Would that be intended as a possible future theme setting?

I thought that it might be pretty easy for both Content Authors and DoccRender Developers using the Row component to build further components to specify the spacing in the future. Its just as easy to provide the gap via css rules, but this just gives a straight forward API.

I actually needed this for #419 but I think the default we added now is good enough for that use case.

[dobromir-hristov]

I may be misunderstanding, but I don't think so. For example, I would expect the following to result in a grid with only 2 columns which are evenly spaced half/half, taking up the full width of the content area:

Almost...

What you explained is exactly how it works now. But what if you add more columns in your scenario? How should it behave? Should it limit you to a max of 12 and wrap? Or should we limit to a min-width-per-column scenario? Or do we not limit them at all 🤣 What if their content just does not fit the space that is available for 1/12 of a column.

@SamLanier and @ethan-kusters I need your input on this, so I can wrap up the styling.

@Row {
  @Column {
    Foo
  }
  @Column {
    Bar
  }
  ... etc to 15
}

[SamLanier]

I may be misunderstanding, but I don't think so. For example, I would expect the following to result in a grid with only 2 columns which are evenly spaced half/half, taking up the full width of the content area:

Almost...

What you explained is exactly how it works now. But what if you add more columns in your scenario? How should it behave? Should it limit you to a max of 12 and wrap? Or should we limit to a min-width-per-column scenario? Or do we not limit them at all 🤣 What if their content just does not fit the space that is available for 1/12 of a column.

@SamLanier and @ethan-kusters I need your input on this, so I can wrap up the styling.

@Row {
  @Column {
    Foo
  }
  @Column {
    Bar
  }
  ... etc to 15
}

We do not want to limit the authoring of columns. Yes, this means authors can create some not-so-great layouts, but it should be up to the author (or authoring guidelines) to create the best experience for the content.

[dobromir-hristov]

@swift-ci test

[dobromir-hristov]

OK I have refactored this to not have limits anymore.

[dobromir-hristov]

@swift-ci test

[marinaaisa]

I tested different use cases and it works great :)

I left minor comments

[marinaaisa]

Why is min-width: 0; needed?

[dobromir-hristov]

Tells the flex/column item to become smaller than it can right now, so it does not overflow.

If we have images or text that are wider than the column, this will make the column outgrow what it should.

[marinaaisa]

Are we using this prop at the moment?

[dobromir-hristov]

Not right now, but I am assuming we may start to, if we start using this as a building block for future components.

[dobromir-hristov]

@swift-ci test

This was addressed.