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

Rename Html:add() to Html:addHtml() #111

Merged
merged 1 commit into from
Jun 17, 2016
Merged

Rename Html:add() to Html:addHtml() #111

merged 1 commit into from
Jun 17, 2016

Conversation

JanTvrdik
Copy link
Contributor

enumag
enumag reviewed May 20, 2016
View reviewed changes
src/Utils/Html.php
* Adds new element's child.
* @param Html|string Html node or raw HTML string
* @return self
*/
public function add($child)
public function addHtml($child)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe only allow Html instance?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure that's a good idea. How would you provide custom HTML as string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You would need to wrap it as an Html instance. But I think that the current solution is better.

@spaze spaze mentioned this pull request May 20, 2016
Merged
@JanTvrdik
Copy link
Contributor Author

Other places which may lead to confusion:

  • $el->insert – maybe not used by anyone directly
  • $el[] – likely used by somebody, don't know how to solve except for deprecating the entire ArrayAccess implementation

milo
milo reviewed May 21, 2016
View reviewed changes
src/Utils/Html.php
@@ -363,17 +363,39 @@ public function getText()


/**
* @deprecated
*/
public function add($child)
Copy link
Member

Choose a reason for hiding this comment

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

I like how is the add() short. What about to allow HTML instance and strings consider as a text? Not sure how to warn about change.

@castamir
Copy link

@JanTvrdik
$el->insert() might became protected or should at least be annotated as @internal
$el[] is a shortcut for adding child elements, isn't it? We can modify array access setter to use addHtml()... or we can deprecate this feature if noone actually use it :D

@dg dg merged commit 1e58e66 into nette:master Jun 17, 2016
dg pushed a commit to nette/forms that referenced this pull request Jun 17, 2016
@spaze @dg
Rename Html:add() to Html:addHtml() to follow nette/utils#111 (#120)
aef9f3a
dg pushed a commit that referenced this pull request Jun 17, 2016
@JanTvrdik @dg
Html: renamed add() to addHtml(); added addText() (#111)
c455ade
dg pushed a commit that referenced this pull request Jun 17, 2016
@JanTvrdik @dg
Html: added addHtml() and addText() (#111)
cdf81ed
dg pushed a commit that referenced this pull request Jun 17, 2016
@JanTvrdik @dg
Html: added addHtml() and addText() (#111)
d372561
dg pushed a commit that referenced this pull request Jun 17, 2016
@JanTvrdik @dg
Html: added addHtml() and addText() (#111)
66ceba1
dg pushed a commit to nette/forms that referenced this pull request Jun 17, 2016
@spaze @dg
Rename Html:add() to Html:addHtml() to follow nette/utils#111 (#120)
a22f01e
@JanTvrdik JanTvrdik deleted the pr/html_add branch June 19, 2016 08:03
northys pushed a commit to northys/utils that referenced this pull request Jul 18, 2016
@JanTvrdik
Html: renamed add() to addHtml(); added addText() (nette#111)
6acefe4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@JanTvrdik @castamir @fprochazka @milo @enumag @dg