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

fix: always use absolute positioning to prevent jumping Orbit slider #11107

Merged
merged 3 commits into from Apr 12, 2018
Merged

fix: always use absolute positioning to prevent jumping Orbit slider #11107

merged 3 commits into from Apr 12, 2018

Conversation

DanielRuf
Copy link
Contributor

Before and after a slide is animted its position attribute is changed between absolute and relative which can produce a jump of the element after a slide change.

Closes #11082

this.options[`animInFrom${dirIn}`],
function(){
$newSlide.css({'position': 'relative', 'display': 'block'})
.attr('aria-live', 'polite');
$newSlide.attr('aria-live', 'polite');
Copy link
Contributor

@ncoden ncoden Apr 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slides display are set to hidden when motion-ui is not used.

https://github.com/DanielRuf/foundation-sites/blob/dbdae96312bcccd0820d8ee43c088d5d6a1fab14/js/foundation.orbit.js#L153

We should keep 'display': 'block';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, just the block property?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, and remove 'position': 'relative' from line 153. But make some tests (with and without motionUI / mui classes), I'm not sure.

@DanielRuf
Copy link
Contributor Author

Ok, will do some more tests later.

@ncoden
Copy link
Contributor

ncoden commented Apr 9, 2018

@DanielRuf Tell me when you done your tests or if I should take care of them 😉

@DanielRuf
Copy link
Contributor Author

or if I should take care of them

Some help is really appreciated and would be great.

…ute" new behavior

All Orbit slides should always have an `absolute` positioning to prevent jumping Orbit slider (see #11107). The `'position': 'relative'` reset is no longer necessary here and break the Orbit behavior as inline styles overrides `.orbit-slide` class styles.

See:
* dbdae96
* #11107
Copy link
Contributor

@ncoden ncoden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, and remove 'position': 'relative' from line 153. But make some tests (with and without motionUI / mui classes), I'm not sure.

I found a bug before figuring out that 'position': 'relative' was not removed. I took care of this.

7daaba5 fix: remove position reset of Orbit slides preventing the "full-absolute" new behavior

All Orbit slides should always have an absolute positioning to prevent jumping Orbit slider (see #11107). The 'position': 'relative' reset is no longer necessary here and break the Orbit behavior as inline styles overrides .orbit-slide class styles.

I made some tests and everything seems to work 👍

@ncoden ncoden added this to the 6.5.0 milestone Apr 11, 2018
@ncoden ncoden merged commit 223ed4a into foundation:develop Apr 12, 2018
@DanielRuf DanielRuf deleted the fix/orbit-slide-positioning-jump-11082 branch April 12, 2018 22:15
ncoden added a commit to ncoden/foundation-sites that referenced this pull request Jun 16, 2018
…tioning-jump-11082 for v6.5.0

dbdae96 fix: always use absolute positioning to prevent jumping Orbit slider
cf80a48 fix: render slide as block element
7daaba5 fix: remove position reset of Orbit slides preventing the "full-absolute" new behavior

Co-Authored-By: Nicolas Coden <nicolas@ncoden.fr>
Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants