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.

Sign up for GitHub

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

Jump to bottom

feat: void element tags in helpers are selectable between > and /> #6789

Merged
merged 10 commits into from
Nov 1, 2022
Merged

feat: void element tags in helpers are selectable between > and /> #6789

merged 10 commits into from
Nov 1, 2022

Conversation

ddevsr
Copy link
Collaborator

@ddevsr ddevsr commented Oct 30, 2022 •
edited by kenjis
Loading

Description
Fixes #6649
Supersedes and closes #6658 and #6717

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

@ddevsr ddevsr requested review from kenjis and paulbalandan October 30, 2022 13:02
@kenjis
Copy link
Member

kenjis commented Oct 30, 2022 •
edited
Loading

The second commit e92116f is not refactoring.
Why don't you combine the first and second commit into one?

The commit message of the first commit c823018 should be fixed, because it is not test code:

tests: void element with xhtml and html5

refactor: make it void element to valid schema HTML

@kenjis kenjis added 4.3 enhancement PRs that improve existing functionalities labels Oct 30, 2022
@ddevsr
Copy link
Collaborator Author

ddevsr commented Oct 30, 2022

@kenjis I can modified CS-Fixer recommendation in this PR? or in other PR already fixed?

@kenjis
Copy link
Member

kenjis commented Oct 30, 2022 •
edited
Loading

Now fixed.
If you rebase, the php-cs-fix errors will be gone.

@paulbalandan
Copy link
Member

@ddevsr please rebase again

paulbalandan
paulbalandan reviewed Oct 30, 2022
View reviewed changes
Copy link
Member

@paulbalandan paulbalandan left a comment

Choose a reason for hiding this comment

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

Some doc rewording comments from me.

user_guide_src/source/changelogs/v4.3.0.rst Outdated Show resolved Hide resolved
user_guide_src/source/installation/upgrade_430.rst Outdated Show resolved Hide resolved
user_guide_src/source/installation/upgrade_430.rst Outdated Show resolved Hide resolved
@kenjis
Copy link
Member

kenjis commented Oct 30, 2022

Thank you @ddevsr and @paulbalandan

totoprayogo1916
totoprayogo1916 reviewed Oct 30, 2022
View reviewed changes
Copy link
Contributor

@totoprayogo1916 totoprayogo1916 left a comment

Choose a reason for hiding this comment

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

Maybe, the HTML Helper needs to be improved too
link_tag etc

@kenjis
Copy link
Member

kenjis commented Oct 30, 2022

link_tag
source
track
param
embed
See https://codeigniter4.github.io/CodeIgniter4/helpers/html_helper.html

@ddevsr
Copy link
Collaborator Author

ddevsr commented Oct 31, 2022

@kenjis Already refactor more html_helper with solidus and small refactor in link_tag

kenjis
kenjis requested changes Oct 31, 2022
View reviewed changes
Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

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

Please update the user guide for HTML helper.

@ddevsr ddevsr requested a review from kenjis October 31, 2022 13:02
@kenjis kenjis requested review from totoprayogo1916 and paulbalandan and removed request for totoprayogo1916 October 31, 2022 23:11
@kenjis kenjis merged commit a7de35a into codeigniter4:4.3 Nov 1, 2022
@ddevsr ddevsr deleted the solidus-html branch November 1, 2022 08:05
@paulbalandan paulbalandan mentioned this pull request Nov 1, 2022
Closed
@kenjis kenjis mentioned this pull request Nov 23, 2022
Merged
5 tasks
@kenjis kenjis added breaking change Pull requests that may break existing functionalities and removed breaking change Pull requests that may break existing functionalities labels Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulbalandan paulbalandan paulbalandan approved these changes

@totoprayogo1916 totoprayogo1916 totoprayogo1916 approved these changes

@kenjis kenjis kenjis approved these changes

Assignees
No one assigned
Labels
4.3 enhancement PRs that improve existing functionalities
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ddevsr @kenjis @paulbalandan @totoprayogo1916