Skip to content
This repository has been archived by the owner on Feb 20, 2018. It is now read-only.

Add ability to specify the size of the space between the element and alignElement #4

Closed
kienD opened this issue Oct 27, 2017 · 6 comments

Comments

@kienD
Copy link

kienD commented Oct 27, 2017

It would be useful to be able to specify the size of the space you would like between the element and alignElement for the Align.getAlignRegion method.

I think this could be done by adding a parameter that accepts a number value to the Align.getAlignRegion method for the spacing size and then add or subtract the value of the spacing parameter to the values in each of the switch statements.

The parameter will probably need to be added to other methods that use Align.getAlignRegion like Align.align as well.

@kienD kienD changed the title Add ability to specific the size of the space between the element and alignElement Add ability to specify the size of the space between the element and alignElement Oct 27, 2017
@robframpton
Copy link

Hey @kienD,

Makes sense to me. Out of curiosity, what's the workaround you guys are currently using?

Also, it seems like it would be easy to add something just to the align method, rather than messing with getAlignRegion.

For example, a param called offset which would look like {top: 2, bottom: -4}, and you could just modify bestRegion with those values here.

@kienD
Copy link
Author

kienD commented Feb 1, 2018

@Robert-Frampton,

We would add an offset to the side of the element that Align.align would return by adding either of the following styles to the element:

`margin-X: ${offset}px`

or

`{transform: translateX(${offset})px}`

One of the issues with this was that it possible to run into the following infinite loop:
1. The element will render near the window edge.
2. The element will try to render at a different position because it's overflowing the window.
3. The element will try to render near the edge again because it was the preferred location.

I'll look into what you mentioned and will send a PR in.

Thanks!

@jwu910
Copy link

jwu910 commented Feb 1, 2018

Once this gets added in, Loop will be able to fix the issue with the indecisive hover modal! Looks cool!

@kienD
Copy link
Author

kienD commented Feb 2, 2018

@jwu910,

I don't think this will fix your issue with the indecisive hover modals.

@Robert-Frampton,

After reading @jwu910's comment, I realized that the implementation suggested above would probably introduce a bug where the element can render off screen if the offset is large enough and if autoBestAlign = true.

I will try to look into a way we can add and account for the offset in the suggestAlignBestRegion calculation as well.

Thanks,

@robframpton
Copy link

Hey @kienD, sounds good. I'm also going to migrate this repo over to metal-plugins, so eventually when you have the right fix you can send it there.

@jbalsas
Copy link

jbalsas commented Feb 19, 2018

Moved to metal/metal-plugins#26

@jbalsas jbalsas closed this as completed Feb 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants