On 02/15/2016 04:20 PM, Pavel Vomacka wrote:
On 02/12/2016 01:52 PM, Pavel Vomacka wrote:
On 02/11/2016 12:31 PM, Pavel Vomacka wrote:
The canvas of the graph had static size. This patch fixes this issue
and from now the graph canvas is resized according to the window size.
Because of changes in previous patch I'm sending also this one again.
Plus I fixed some jslint warnings.
And again a link to the ticket:
And another change in the code. This patch adds checking whether a svg
element even exists. And don't add 'col-sm-12' class to the svg element
any more. This class just added useless paddings to the element.
thanks for the patch.
1. I don't like the fact that the resize handler registered in
initialize method is active forever, even when viewing other facets.
2. The code will probably fail if there is other svg element present on
$('svg') searches for all svg elements in DOM, such search is usually
slow and undeterministic. It is better to use a stored reference(if
possible) or limit the search to some parent element, e.g. TopoGraph can
store and then use its container.
Would be funny if there were 2 graphs.
3. Why is there the toFixed(1) call? Or more specifically on that
position? It hides the fact that toFixed transforms Number to String and
then '-' operator with Number on the right casts it back to Number.
4. width could be just: this._svg.parent().width()
5. Your approach for bottom padding works well but I don't like that the
component assumes that there is some col-sm-12 element on a page whose
right padding is actually equal to space on the left of the svg.
#1 and #5 makes me think that the resize logic should be moved topology
facet. Something like:
* register resize handler on facet's 'show' event
* unregister resize handler on facet's 'hide' event (will solve #1)
* on window resize, compute the size in topology facet, call new
.resize(width, height) method of TopoGraph
Then, we wouldn't have to search whole DOM for 'svg' elements to check
if page is visible. The bottom padding can be obtained by:
parseInt(this.content.css('paddingLeft')) where 'this' is facet.
Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code