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.

Sign up for GitHub

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

Jump to bottom

[Visibility] Wrong minimum height set for placeholder element #296

Closed
donaggio opened this issue Dec 11, 2018 · 2 comments
Closed

[Visibility] Wrong minimum height set for placeholder element #296

donaggio opened this issue Dec 11, 2018 · 2 comments
Assignees
@y0hami
Labels
lang/javascript Anything involving JavaScript type/bug Any issue which is a bug or PR which fixes a bug
Milestone
2.7.0

Comments

@donaggio
Copy link

Bug Report

Description

When a .ui.segment is made fixed using the visibility API, a placeholder element is created

to ensure that the page's position does not change when the element is "fixed" and removed from normal layout flow

Expected result

The placeholder element should have the same height of the "fixed" element

Actual result

The placeholder element has a min-height: 18em CSS property which makes it a lot taller then needed in the majority of cases

Testcase

https://jsfiddle.net/m97k6rqw/1/

Version

2.6.4
@ColinFrick ColinFrick added type/bug Any issue which is a bug or PR which fixes a bug lang/javascript Anything involving JavaScript labels Dec 11, 2018
@ColinFrick
Copy link
Member

The visibility API uses the class name placeholder, which is now used in the new 2.4 placeholder module. Changing the placeholder class name in visibility.js should fix this issue:
https://jsfiddle.net/73ysw1zp/

@ColinFrick ColinFrick added the tag/good-first-issue Good issues for new starters to try label Dec 11, 2018
@y0hami y0hami added this to the 2.7.0 milestone Dec 11, 2018
@y0hami
Copy link
Member

y0hami commented Dec 11, 2018

We will do this for 2.7 since it is an API breaking.

@y0hami y0hami self-assigned this Dec 11, 2018
y0hami pushed a commit that referenced this issue Dec 11, 2018
fix(visibility): rename placeholder class to constraint
99d179d 
Rename the placeholder class to constraint to stop the
conflict with the placeholder component

Closes #296
@y0hami y0hami mentioned this issue Dec 11, 2018
Merged
@y0hami y0hami added the tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build label Dec 11, 2018
y0hami pushed a commit that referenced this issue Dec 11, 2018
fix(visibility): rename placeholder class to constraint
fba5c8c 
Rename the placeholder class to constraint to stop the conflict with the placeholder component

Closes #296
@lubber-de lubber-de removed the tag/good-first-issue Good issues for new starters to try label Dec 11, 2018
y0hami pushed a commit that referenced this issue Dec 18, 2018
fix(visibility): rename placeholder class to constraint
e0993c7 
Rename the placeholder class to constraint to stop the conflict with the placeholder component

Closes #296
This was referenced Dec 21, 2018
Merged
Merged
@y0hami y0hami closed this as completed in 7fb8a5c Dec 21, 2018
@lubber-de lubber-de removed the tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build label Dec 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@y0hami y0hami

Labels
lang/javascript Anything involving JavaScript type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Milestone
2.7.0
Development

No branches or pull requests
4 participants
@ColinFrick @y0hami @donaggio @lubber-de