Skip to content
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 multi line cell and longer $listPrefix in Table widget #19906

Merged
merged 24 commits into from
Jul 24, 2023

Conversation

rhertogh
Copy link
Contributor

@rhertogh rhertogh commented Jul 21, 2023

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues -

Currently a multi line string breaks the table output, this PR fixes this.

From the unit test,
Current output:

╔═══════════════╤════════════════╤═════════════════╗
║ test1         │ test2          │ test3
multiline ║
╟───────────────┼────────────────┼─────────────────╢
║ test
content1 │ testcontent2   │ test
content3   ║
╟───────────────┼────────────────┼─────────────────╢
║ testcontent21 │ test
content22 │ testcontent23   ║
╚═══════════════╧════════════════╧═════════════════╝

Fixed output:

╔═══════════════╤══════════════╤═══════════════╗
║ test1         │ test2        │ test3         ║
║               │              │ multiline     ║
╟───────────────┼──────────────┼───────────────╢
║ test          │ testcontent2 │ test          ║
║ content1      │              │ content3      ║
╟───────────────┼──────────────┼───────────────╢
║ testcontent21 │ test         │ testcontent23 ║
║               │ content22    │               ║
╚═══════════════╧══════════════╧═══════════════╝

This PR also fixes a bug when $listPrefix doesn't have an exact length of 2 characters, for example '-- '.

@what-the-diff
Copy link

what-the-diff bot commented Jul 21, 2023

PR Summary

  • Improved Handling of Multiline Cell Contents in Tables
    The team added an additional feature to the 'Table' class that checks for and handles content that spans multiple lines within a single cell. This ensures that our data tables are displayed accurately and neatly, no matter the length of the content inside a cell.

  • Testing of Multiline Cell Contents in Tables
    A new test case has been integrated within the 'TableTest' class that verifies the proper rendering of tables where cell contents span multiple lines. This helps to guarantee that our new feature works seamlessly and as intended, ensuring reliability for users.

  • Updated 'TableTest' Class
    The 'TableTest' class has been updated to include this new test case, which specifically ensures that a table with multiline cell contents can be rendered correctly. This continue to streamline and advance our testing module, ensuring the highest quality output in our table rendering capabilities.

@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Patch coverage: 75.00% and no project coverage change.

Comparison is base (76d6345) 48.84% compared to head (1d8be31) 48.85%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #19906   +/-   ##
=======================================
  Coverage   48.84%   48.85%           
=======================================
  Files         445      445           
  Lines       42763    42781   +18     
=======================================
+ Hits        20888    20901   +13     
- Misses      21875    21880    +5     
Impacted Files Coverage Δ
framework/console/widgets/Table.php 71.75% <75.00%> (+0.04%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rhertogh rhertogh marked this pull request as ready for review July 21, 2023 21:56
@rhertogh rhertogh changed the title Fix multi line cell in Table widget Fix multi line cell and longer $listPrefix in Table widget Jul 22, 2023
@rhertogh
Copy link
Contributor Author

@bizley FYI: I've made an additional commit after your review: 1d8be3149
It fixes rendering of long multiline strings and longer $listPrefix in \yii\console\widgets\Table.

@samdark samdark merged commit 1a0e91e into yiisoft:master Jul 24, 2023
49 checks passed
@samdark
Copy link
Member

samdark commented Jul 24, 2023

👍

@samdark samdark added this to the 2.0.49 milestone Jul 24, 2023
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.

4 participants