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

{center: true} does not work correctly with {top: xx} in arrays and trees #179

Open
cashaffer opened this issue Dec 28, 2014 · 6 comments
Open

Comments

@cashaffer
Copy link
Collaborator

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.

@vkaravir
Copy link
Owner

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.

@cashaffer
Copy link
Collaborator Author

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.

@vkaravir
Copy link
Owner

vkaravir commented Jan 7, 2015

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 top and center to move down a centered structure. This, again, could break something existing. Should I still change it?

@cashaffer
Copy link
Collaborator Author

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

{top: 50, left: centered}

or perhaps

{top: 50, horizontal: center}

Though making "center" be the default does seem reasonable.

@vkaravir
Copy link
Owner

vkaravir commented Jan 7, 2015

I didn't plan on adding any new options (there are quite many already). I thought I'd change the behavior to be:

  • option {center: true} would center the structure
  • options {center: true, top: 30} would center the structure and move it 30px down. Currently this would position it 30px from the top of the container.
  • option {top: 30} would behave like above, since center is true by default.
  • options {top: 30, left: 10} would position the structure 30px from the top and 10px from the left of the container (like it does currently).

Would this solve the problem you originally reported in this issue?

@cashaffer
Copy link
Collaborator Author

Yes, that sounds ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants