-
Notifications
You must be signed in to change notification settings - Fork 41
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
{center: true} does not work correctly with {top: xx} in arrays and trees #179
Comments
I've thought of this before and decided that it's not a bug. Setting top, bottom, left, or right option for the constructor sets the structure positioning into absolute mode. So it will use the absolute positioning of the browser based on the positioning properties (top, bottom, etc). The alternative way to implement this would be to use top to move the structure relatively if left/right is not specified. However, I find this less intuitive than always switching to absolute positioning if any of the positioning properties are specified. Or do you think it should still center the structure if left/right is not specified? Btw, if your intention is to move the structure down, setting top margin would be the way to go. |
It is quite common that I want to adjust the height of an array or tree, but I want it centered. It seems like an unfortunate side effect that setting the vertical positioning would affect the horizon positioning. How should I set the horizontal positioning to center again if I set the top? I don't see how to set the top margin. There doesn't seem to be an option for it in the constructor (like there is for top). And I want to move individual arrays or trees, so I am not sure how to do it with CSS. |
I looked at the code and it should be relatively easy to change the positioning to work so that you could use the combination of |
This will be an issue of what options are given in the constructor for the tree or array object, right? If I am clear on what would need updated, I can do that easily enough for the various AVs. We have a lot of trouble now with things not being centered correctly, so it would not be so bad to do some house cleaning. I could live with a solution that required me to explicitly give an option. Such as
or perhaps {top: 50, horizontal: center} Though making "center" be the default does seem reasonable. |
I didn't plan on adding any new options (there are quite many already). I thought I'd change the behavior to be:
Would this solve the problem you originally reported in this issue? |
Yes, that sounds ideal. |
Normally, arrays and trees default to {center: true} on their layout. However, if you add {top: 10} to the constructor, the default horizontal centering gets lost. Not only that, but explicitly adding {center: true} will not have any effect.
AV/General/UFfigCON.js illustrates the problem. This diagram appears as the first figure of the UnionFind module: http://algoviz.org/OpenDSA/dev/OpenDSA/Books/CS3114/html/UnionFind.html.
The text was updated successfully, but these errors were encountered: