Skip to content
This repository has been archived by the owner on May 2, 2023. It is now read-only.

feat(table): Extra attributes and extra classes for the table rows - TWIG-203 #305

Merged
merged 4 commits into from
Jan 28, 2020

Conversation

planctus
Copy link
Contributor

PR description

Please drop a few lines about the PR: what it does, how to test it, etc.

QA Checklist

In order to ensure a safe and quick review, please check that your PR follow those guidelines:

  • I have put the vanilla component as devDependencies
  • I have put the specs package as devDependencies
  • I have added the components directly used in the twig file (with include or embed) as dependencies
  • My component is listed in @ecl-twig/ec-components's dependencies
  • My variables naming follow the guidelines (snake case for twig)
  • I have provided tests
  • I have provided documentation (for the "notes" tab)
  • If my local yarn.lock contains changes, I have committed it
  • I have given my PR the proper label (pr: review needed to indicate that I'm done and now waiting for a review ,pr: wip to indicate that I'm actively working on it ...)

@planctus planctus added the pr: review needed Use this label to show that your PR needs to be review label Jan 24, 2020
@papegaill papegaill added pr: under review and removed pr: review needed Use this label to show that your PR needs to be review labels Jan 27, 2020
"label" (string or array of string)
"data-ecl-table-header" (string) (default: ''),
"group" (bool) (default: false),
},
...
]
- "extra_classes" (string) (default: '')
- "extra_attributes" (array) (default: []): format: [
- "extra_attributes" (string) (default: []): format: [
Copy link
Contributor

Choose a reason for hiding this comment

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

Main extra_attributes should stay as an array? Also, for rows, why not keeping same array structure, how to do if you want to pass an extra_attribute with a value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right about the fact that i also changed the casting for the main "extra_attributes" and that is unwanted. But for the "new" extra_attributes for the rows, in order to use the same approach we have for the main attributes we should also apply the same logic to them, if it comes as an array we have to process it and build a string out of it. This is also happening in a for loop in this case, so putting all this processing there looks really overwhelming, if you have the value you pass the full attribute (name="value"), anyway.. ;)

@planctus planctus added pr: review needed Use this label to show that your PR needs to be review and removed pr: modification needed labels Jan 28, 2020
@papegaill papegaill removed the pr: review needed Use this label to show that your PR needs to be review label Jan 28, 2020
@papegaill papegaill merged commit 9449791 into develop Jan 28, 2020
@papegaill papegaill deleted the TWIG-203 branch January 28, 2020 14:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants