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: standardize behavior of make:cell and Cells #7481

Merged
merged 3 commits into from
May 21, 2023

Conversation

paulbalandan
Copy link
Member

Description
Fixes #7472
Fixes #7469

Supersedes and closes #7479
Supersedes and closes #7471

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@paulbalandan paulbalandan added the bug Verified issues on the current code behavior or pull requests that will fix them label May 5, 2023
@kenjis
Copy link
Member

kenjis commented May 9, 2023

This behavior does not match the current specification in the docs.

So, for example, if you have a MyCell class, the view file should be my_cell.php.
https://codeigniter4.github.io/userguide/outgoing/view_cells.html#controlled-cells

In my understanding, the current specification is:

  1. class: MyCell view: my_cell.php (default)
  2. class: MyCell view: my.php (fallback; undocumented)

There is inconsistency between make:cell and the docs.
It will bring confusion or bug reports.

@paulbalandan
Copy link
Member Author

Based on @lonnieezell 's comment, the specification should be:

  1. class: MyCell, view: my.php (default)
  2. class: MyCell, view: my_cell.php (fallback for BC)

That part of the docs should be updated, too.

@kenjis
Copy link
Member

kenjis commented May 9, 2023

If we change the specification, add the following in this PR:

  1. update the View Cells page's descriptions and sample code
  2. add description that we accept "class: MyCell view: my_cell.php"

Also, it would be nice to have an option to generate _cell.php views, but this can be added later.

@kenjis kenjis added the stale Pull requests with conflicts label May 14, 2023
Copy link
Member

@lonnieezell lonnieezell left a comment

Choose a reason for hiding this comment

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

With the exception of the one item below this looks good to me.

user_guide_src/source/outgoing/view_cells.rst Outdated Show resolved Hide resolved
system/View/Cells/Cell.php Outdated Show resolved Hide resolved
system/View/Cells/Cell.php Outdated Show resolved Hide resolved
@kenjis kenjis removed the stale Pull requests with conflicts label May 18, 2023
@kenjis kenjis merged commit 61df4df into codeigniter4:develop May 21, 2023
@kenjis
Copy link
Member

kenjis commented May 21, 2023

@paulbalandan Thank you!

@paulbalandan paulbalandan deleted the standard-make-cell branch May 21, 2023 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
4 participants