-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: HTML Table data keys synchronize order with Heading keys #7409
Merged
Merged
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
c7b02d8
data row columns optional order by heading keys
rumpfc f420ad9
Docu, CS fixes
rumpfc bde7c42
More CS fixes (trailing comma in array)
rumpfc 90dfeee
Docu update with more example
rumpfc 012bbf6
Fixing CS in docu
rumpfc d6099ff
Fixing CS in docu
rumpfc 0e4f793
Fixing CS in docu
rumpfc 3bfd3b8
Renamed variable and method name
rumpfc 4a9bb60
CS fixes after change
rumpfc ddb6d3f
Added to changelog and suggested changes
rumpfc 7543912
CS fix
rumpfc 5405ba4
Fixed typos
rumpfc 524e880
Minor updates
rumpfc 57fc049
Merge branch '4.4' into feature-view-table-order
kenjis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -87,6 +87,7 @@ Others | |||||
See :ref:`controller-default-method-fallback` for details. | ||||||
- **Filters:** Now you can use Filter Arguments with :ref:`$filters property <filters-filters-filter-arguments>`. | ||||||
- **Request:** Added ``IncomingRequest::setValidLocales()`` method to set valid locales. | ||||||
- **Table:** Addedd ``Table::setSyncRowsWithHeading()`` method to synchronize row columns with heading. See :ref:`table-sync-rows-with-headings` for details. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
Message Changes | ||||||
*************** | ||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
<?php | ||
|
||
$table = new \CodeIgniter\View\Table(); | ||
|
||
$table->setHeading(['name' => 'Name', 'color' => 'Color', 'size' => 'Size']) | ||
->setSyncRowsWithHeading(true) | ||
->addRow(['color' => 'Blue', 'name' => 'Fred', 'size' => 'Small']) | ||
->addRow(['size' => 'Large', 'age' => '24', 'name' => 'Mary']) | ||
->addRow(['color' => 'Green']); | ||
|
||
echo $table->generate(); | ||
?> | ||
|
||
<!-- Generates a table with this prototype: --> | ||
<table border="0" cellpadding="4" cellspacing="0"> | ||
<thead> | ||
<tr> | ||
<th>Name</th> | ||
<th>Color</th> | ||
<th>Size</th> | ||
</tr> | ||
</thead> | ||
<tbody> | ||
<tr> | ||
<td>Fred</td> | ||
<td>Blue</td> | ||
<td>Small</td> | ||
</tr> | ||
<tr> | ||
<td>Mary</td> | ||
<td></td> | ||
<td>Large</td> | ||
</tr> | ||
<tr> | ||
<td></td> | ||
<td>Green</td> | ||
<td></td> | ||
</tr> | ||
</tbody> | ||
</table> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
|
||
$data = [ | ||
[ | ||
'color' => 'Blue', | ||
'name' => 'Fred', | ||
'size' => 'Small', | ||
], | ||
[ | ||
'size' => 'Large', | ||
'age' => '24', | ||
'name' => 'Mary', | ||
], | ||
[ | ||
'color' => 'Green', | ||
], | ||
]; | ||
|
||
$table = new \CodeIgniter\View\Table(); | ||
|
||
$table->setHeading(['name' => 'Name', 'color' => 'Color', 'size' => 'Size']) | ||
->setSyncRowsWithHeading(true); | ||
|
||
echo $table->generate($data); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a need to make this public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All other properties are
public
, I will keep it for consistencyThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All other properties seems to be wrong, but changing them is breaking changes...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paulbalandan
public
for current consistencyprotected
orprivate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like this new property to be
private
to enforce API encapsulation. We can increase visibility later if needed.For the others, we can retain them as
public
to prevent breaking changes.