-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
Fix #14: Use ul
and li
tags for the default layout of the ListView
#210
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #210 +/- ##
============================================
+ Coverage 64.66% 68.07% +3.41%
- Complexity 558 569 +11
============================================
Files 46 45 -1
Lines 1862 1848 -14
============================================
+ Hits 1204 1258 +54
+ Misses 658 590 -68 ☔ View full report in Codecov by Sentry. |
Are the missing quotes a desired thing on
This expected output for the test is the result of rendering https://github.com/yiisoft/yii-dataview/blob/848da41ead3407cea7a3647e8db66058d9a33708/tests/Support/view/_listviewparams.php which don't have the quotes. |
…14-listview-and-semantics
…14-listview-and-semantics
…14-listview-and-semantics
…items wrapper element.
WIP: Need to make the LI tag also configurable for the |
@@ -76,12 +80,14 @@ public function testItemViewAsString(): void | |||
Assert::equalsWithoutLE( | |||
<<<HTML | |||
<div> | |||
<div> | |||
<ul> | |||
<li> | |||
<div>Id: 1</div><div>Name: John</div><div>Age: 20</div> |
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 <span>
is more suitable here considering <li>
.
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.
We need to separate items somehow. With div
is break line.
src/ListView.php
Outdated
} | ||
|
||
return Div::tag() | ||
return Li::tag() |
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.
Item tag should be customizeable also.
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.
Yes, I'm working on it
@glpzzz sounds like a good idea. |
I agree with @samdark. See as example: Line 705 in 848da41
Suggest implement it in separate PR only. |
Tests also modified. The extra \n is not a problem and expected.
ul
and li
tags for the default layout of the ListView
.ul
and li
tags for the default layout of the ListView
👍 |
As per commented on #14,
ul
andli
tags are used for the default layout of the ListView.