Skip to content

Conversation

@rmccar
Copy link
Contributor

@rmccar rmccar commented Sep 26, 2025

What is the context of this PR?

Fixes: ONSDESYS-653

This PR fixes some bugs in the table macro related to some logic in that macro that doesnt work correctly in a python/jinja2 environment.
The first change is to the way the ons-table--has-rowspan is added to the <table> element. In jinja2 any variable set inside a for loop are only accessible in that for loop so the changes to the hasRowSpan var were inaccessible from outside of the loop so it was always false when it came to adding the class. This had to be refactored to add the class inside the loop instead of setting the var. This fixed the spacing and border issues.
The second change fixes the way keys are accessed and set within object in a jinja2 env. Without the quotes around ths this was being set as undefined and therefore in the loop {% for th in headerCols.ths %} this key was inaccessible so the titles were never being set.

How to review this PR

  • spin up the design-system-python-flask-demo repo and see that the column titles are missing, the spacing is off on the second column and the bottom border does not span the width of the table in the example-table-merge-cells example
  • manually copy the table macro from this branch into the design-system-python-flask-demo repo
  • remove the load-design-system-templates step from the run command in the make file
  • spin up that repo again and see that these issues are resolved
  • do the same steps in the DS repo to make sure nothing is broken or there aren't any unintended changes there

Checklist

This needs to be completed by the person raising the PR.

  • I have selected the correct Assignee
  • I have linked the correct Issue

@rmccar rmccar self-assigned this Sep 26, 2025
@rmccar rmccar requested a review from a team as a code owner September 26, 2025 13:41
@rmccar rmccar added the Bug Something isn't working label Sep 26, 2025
@netlify
Copy link

netlify bot commented Sep 26, 2025

Deploy Preview for ons-design-system-preview ready!

Name Link
🔨 Latest commit c579659
🔍 Latest deploy log https://app.netlify.com/projects/ons-design-system-preview/deploys/68dd242f5a985d00098180b0
😎 Deploy Preview https://deploy-preview-3739--ons-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@SriHV SriHV merged commit 6f00ab6 into main Oct 2, 2025
15 checks passed
@SriHV SriHV deleted the fix-python-merged-table-rendering branch October 2, 2025 12:57
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

Successfully merging this pull request may close these issues.

4 participants