Skip to content

Conversation

@QuietMisdreavus
Copy link
Contributor

Bug/issue #, if applicable: rdar://100337272

Summary

This PR takes a new alignments property from Render JSON and adds new styles to table cells to control their text alignments.

Dependencies

swiftlang/swift-docc#389

Testing

See the testing instructions from the linked Swift-DocC PR.

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
  • [ n/a ] Updated documentation if necessary

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

Copy link
Contributor

@dobromir-hristov dobromir-hristov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great. Left a small comment, which may not be a problem, seeing as the snapshots have been created properly.

Comment on lines 130 to 133
const cellAttrs = attrs;
const align = alignments[cellIndex] || TableColumnAlignments.unset;
if (align !== TableColumnAlignments.unset) cellAttrs.class = `${align}-cell`;
return createElement(element, { attrs: { ...cellAttrs, colspan, rowspan } }, (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think classes have their own dedicated key they should be passed through - https://v2.vuejs.org/v2/guide/render-function.html#The-Data-Object-In-Depth

Suggested change
const cellAttrs = attrs;
const align = alignments[cellIndex] || TableColumnAlignments.unset;
if (align !== TableColumnAlignments.unset) cellAttrs.class = `${align}-cell`;
return createElement(element, { attrs: { ...cellAttrs, colspan, rowspan } }, (
const align = alignments[cellIndex] || TableColumnAlignments.unset;
let classes = null;
if (align !== TableColumnAlignments.unset) classes = `${align}-cell`;
return createElement(element, { attrs: { ...attrs, colspan, rowspan }, class: classes }, (

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks nicer! I was wondering if there was a specific property for CSS classes, but i didn't dig deep enough to see it. I've pushed up this change. Thanks!

@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please test

@QuietMisdreavus QuietMisdreavus merged commit c2ce368 into swiftlang:main Sep 26, 2022
@QuietMisdreavus QuietMisdreavus deleted the table-align branch September 26, 2022 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants