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

[stable13] Fix controls position in gallery layout #405

Merged
merged 2 commits into from Mar 7, 2018

Conversation

Projects
None yet
2 participants
@danxuliu
Member

danxuliu commented Mar 7, 2018

Backport of #404

danxuliu added some commits Mar 6, 2018

Add controls to #app instead of before #content-wrapper
The "#content-wrapper" element is the scrolling container of the
gallery. As the "#controls" element was a sibling with a relative top
position it overlapped the "#content-wrapper"; the contents themselves
were not overlapped due to the "margin-top" CSS rule for "#content
.hascontrols" elements, but as the scroll bar belongs to the
"#content-wrapper" it was.

This could have been fixed by setting the top position of the
"#content-wrapper" element to visually move it below the "#controls".
However, conceptually the controls are part of the contents, and other
apps may expect that the gallery adds all its contents in the
"#content-wrapper" (like the JSXC app), so also for consistency with the
Files app this commit moves the "#controls" element into the "#app"
element instead.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Remove margin from server for "hascontrols" CSS class
The server adds a "margin-top" to the elements with the "hascontrols"
CSS class, like the element for the gallery. However, since the controls
were moved into the "#app" element that margin is no longer necessary;
the controls are a previous sibling of the gallery, and its sticky
position behaves like a relative position before getting stuck, so the
controls are positioned according to the normal flow of the document and
thus "push" the gallery down without the need of a margin.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

@danxuliu danxuliu added this to the Nextcloud 13.0.1 milestone Mar 7, 2018

@danxuliu danxuliu requested review from rullzer, MorrisJobke, oparoz and skjnldsv Mar 7, 2018

@codecov

This comment has been minimized.

codecov bot commented Mar 7, 2018

Codecov Report

Merging #405 into stable13 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             stable13     #405   +/-   ##
===========================================
  Coverage       82.27%   82.27%           
  Complexity        335      335           
===========================================
  Files              38       38           
  Lines            1286     1286           
===========================================
  Hits             1058     1058           
  Misses            228      228

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d6eaf6...90067ff. Read the comment docs.

@danxuliu danxuliu merged commit eb81c87 into stable13 Mar 7, 2018

5 checks passed

codecov/patch Coverage not affected when comparing 7d6eaf6...90067ff
Details
codecov/project 82.27% remains the same compared to 7d6eaf6
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/drone/push the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@danxuliu danxuliu deleted the stable13-404-fix-controls-position-in-gallery-layout branch Mar 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment