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

Tracy\Bar's output can be disabled (closes #82) #83

Closed
wants to merge 2 commits into from

Conversation

@hranicka
Copy link
Contributor

hranicka commented Nov 22, 2014

Closes #82

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented Nov 22, 2014

+1, although public $enabled = TRUE is probably enough

Btw: the commit message should probably be: "Bar: output can be disabled [Closes #82]"

@@ -55,11 +58,38 @@ public function getPanel($id)


/**
* Disables Bar output.
* @param bool

This comment has been minimized.

Copy link
@JanTvrdik

JanTvrdik Nov 22, 2014

Contributor

Both functions take no arguments

@hranicka

This comment has been minimized.

Copy link
Contributor Author

hranicka commented Nov 22, 2014

Thanks @JanTvrdik , I've changed the commit message and fixed PhpDoc.


@JanTvrdik
I prefer public methods (::disable/::enable) instead of public property because in the future Bar can be interface and PHP interface cannot define public properties.

But I can change it to only one method eg. public function enable($enabled = TRUE).
What do you think about it?

@dg

This comment has been minimized.

Copy link
Member

dg commented Nov 25, 2014

What about simple method hide?

@hranicka

This comment has been minimized.

Copy link
Contributor Author

hranicka commented Nov 25, 2014

I've replaced the two methods with one hide($hidden = TRUE).

But "disable" sounds me better than "hide"... maybe...

And I also thinking about option for enabling the Bar again when is disabled. Maybe it's not necessary to have this option.

@dg

This comment has been minimized.

Copy link
Member

dg commented Nov 25, 2014

Maybe disable sound better… The hide was only idea to discuss.

@EdaCZ

This comment has been minimized.

Copy link
Contributor

EdaCZ commented Dec 21, 2014

Whats the status of this, please?

@bazo

This comment has been minimized.

Copy link

bazo commented Jan 3, 2015

please merge this. disable/enable or hide/show are both good names. but it's a pain in the ass to develop a rest api with the bar output in the response

@dg

This comment has been minimized.

Copy link
Member

dg commented Jan 3, 2015

@bazo Send the correct Content-Type header.

@bazo

This comment has been minimized.

Copy link

bazo commented Jan 3, 2015

ok, that helped. but still, this can be a useful feature

@dg dg force-pushed the nette:master branch 11 times, most recently from 9151150 to 00083f3 Feb 9, 2015
@dg dg force-pushed the nette:master branch 4 times, most recently from 0203000 to 0203000 Feb 18, 2015
@encero

This comment has been minimized.

Copy link

encero commented May 22, 2015

+1, i am already using forked version with this merged.

@hranicka

This comment has been minimized.

Copy link
Contributor Author

hranicka commented May 22, 2015

Wait for v2.4 :)

@dg dg force-pushed the nette:master branch from f4f13d8 to 713a85b May 25, 2015
@dg dg force-pushed the nette:master branch from 3da92fa to cdd1741 Jun 19, 2015
@dg dg force-pushed the nette:master branch 3 times, most recently from 13f9a7b to 0c0f9d4 Jul 11, 2015
@dg dg force-pushed the nette:master branch 2 times, most recently from 3495a54 to 8517fdc Oct 1, 2015
@dg dg force-pushed the nette:master branch from e5997c3 to 665c9a8 Oct 19, 2015
@dg dg force-pushed the nette:master branch 7 times, most recently from 5bee3a1 to 42ed404 Nov 4, 2015
@dg dg closed this in 622221f Jan 11, 2016
dg added a commit that referenced this pull request Jan 11, 2016
dg added a commit that referenced this pull request Jan 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.