Skip to content
This repository has been archived by the owner on Dec 25, 2023. It is now read-only.

Refactor box model #335

Merged
merged 66 commits into from
Nov 26, 2022
Merged

Refactor box model #335

merged 66 commits into from
Nov 26, 2022

Conversation

nagmat84
Copy link
Collaborator

Disclaimer

This is a follow-up of #323 and #334. This PR is strictly ahead of those both PRs and hence shows all commits and diffs which are already contained in #323 and #334. Hence, the diff of this PR will "shrink" after those both PRs have been merged.

Summary

This PR revamps the box model of the Lychee frontend such that the box model follows the visual layout of the frontend more closely. In doing so it also fixes two long-lasting issues:

  • The wrong position of the vertical scroll bars is fixed
  • The height of the footer is now always correctly taken into account even for "huge" footers.

Screenshots to illustrate the issues

New scrollbars

As you can see, the scrollable content part of the view and the sidebar have independent vertical scrollbars on their respective right edge.

01 Screenshot_20221019_175158

Correct height of footer

If the footer is large (i.e. every possible option is enabled), the last photo is fully visible and the footer is below it

02 Screenshot_20221019_175517

Previous erroneous behavior on current master

  • The scrollbar of the content view is at the very outer right while the scrollbar of the right sidebar ist left of it. This is very confusing. In particular it was possible to accidentally scroll the "wrong" area.
  • The lower half of the last photo is covered by the footer (the overlay is not visible anymore)
  • The right half of the footer is covered by the sidebar (you might consider this okay'ish, if you think of the sidebar as an overlay)

03 Previous

Under the hood

  • New box model, the boxes are now organized as follows
    04 Box Model
    The illustration is only meant as a helper to understand what has changed.
  • The new box model allows to get rid of JS code which previously attempted to dynamically calculate certain sizes (such as the size of the footer) and to get rid of a lot of CSS classes (such as the CSS class which adopted the right padding of the content area depending on whether the sidebar was open or closed). Hence, the new code should be a lot less error-prone.
  • Previously, the <body> was the element which (erroneously) contained the vertical scrollbar for the scrollable content area. (That was also the reason, why the scrollbar appeared at the very right side.) Now the respective <div> is scrollable; in particular the <body> is always fully visible. This also allowed to remove the JS package scrolllock. Previously, this module was used to keep certain elements in sight even when the body was scrolled.

nagmat84 added 22 commits October 19, 2022 18:10
@nagmat84
Created unified HTML and Gulp task
911fd09
@nagmat84
Remove CSS class "view".
022a71e
@nagmat84
Add path for view mode to lychee.load
5da16c0
@nagmat84
Fixed invocation of prettier
287203a
@nagmat84
Removed "view" from build script and sources.
6350c9c
@nagmat84
Remove page from scripts and styles
2e35054
@nagmat84
Removed unnecessary variables from gulpfile
6f34176
@nagmat84
Removed resolved TODOs from html
b2262ce
@nagmat84
Fix localization of info panel
03c3d67
@nagmat84
Fixed translation and visibility of elements for gallery and view mode.
2b85a1b
@nagmat84
Fixed NSFW warning.
a017486
@nagmat84
Injecting SVG into HTML
065b01d
@nagmat84
Removed resolved TODO for view mode
1b8862d
@nagmat84
Enabled frame mode within frontend.
5a78a8a
@nagmat84
Enabled frame mode (leaving frame mode doesn't work).
6ddf88e
@nagmat84
Cleaned up Gulp file
1bc6a0d
@nagmat84
Make frame mode leavable again.
b302344
@nagmat84
Check whether frame mode is enabled
6a57359
@nagmat84
Enforce non-executable compilation results
b35a6c8
@nagmat84
Make pretty
7a44af3
@nagmat84
Re-enable TV styles
894fbdf
@nagmat84
Update dependencies
52f2ecc
@nagmat84 nagmat84 requested a review from a team October 19, 2022 17:37
@nagmat84 nagmat84 mentioned this pull request Oct 19, 2022
Merged
@ildyria
Copy link
Member

ildyria commented Oct 20, 2022

Under the hood

  • New box model, the boxes are now organized as follows
    04 Box Model
    The illustration is only meant as a helper to understand what has changed.
  • The new box model allows to get rid of JS code which previously attempted to dynamically calculate certain sizes (such as the size of the footer) and to get rid of a lot of CSS classes (such as the CSS class which adopted the right padding of the content area depending on whether the sidebar was open or closed). Hence, the new code should be a lot less error-prone.
  • Previously, the <body> was the element which (erroneously) contained the vertical scrollbar for the scrollable content area. (That was also the reason, why the scrollbar appeared at the very right side.) Now the respective <div> is scrollable; in particular the <body> is always fully visible. This also allowed to remove the JS package scrolllock. Previously, this module was used to keep certain elements in sight even when the body was scrolled.

May I request this to be added to Lychee Docs ? :)

@nagmat84 nagmat84 mentioned this pull request Oct 20, 2022
Merged
nagmat84 and others added 21 commits November 1, 2022 11:35
ildyria
ildyria reviewed Nov 21, 2022
View reviewed changes
Copy link
Member

@ildyria ildyria left a comment

Choose a reason for hiding this comment

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

Quickly tested. LGTM.

@ildyria ildyria merged commit 7991d49 into master Nov 26, 2022
@ildyria ildyria deleted the refactor_box_model branch November 26, 2022 17:12
@nicokaiser nicokaiser mentioned this pull request Feb 5, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ildyria ildyria ildyria approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nagmat84 @ildyria @kamil4