This repository has been archived by the owner on Nov 1, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 58
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The standard layout in the server uses "#app-navigation", "#app-content" and "#app-sidebar" as children of "#content"; the navigation and sidebar are not needed, so "#app" is simply renamed to "#app-content". Also note that "#controls" is already the first child of "#app-content", so there is no need to prepend it. In a similar way there are standard CSS rules defined in the server for "#controls" as a child of "#app-content", so no special rules need to be defined here. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
In the layout provided by the server the scrolling container is now the body instead of the content wrapper. Note that the container used in "galleryfileaction.js" was not modified on purpose, as that feature is currently broken; it restores the scroll position when the slideshow is hidden due to the file list being reloaded, but that does not happen in Files app, only in public pages. Thus, in Files app it restores the position as soon as the file list is reloaded after closing the slideshow, so it changes the scroll position, for example, when the directory changes. However, as "#app-content" is used as the scrolling container in that case the scroll position of the body does not actually change. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Note that the proper replacemente for "#content-wrapper" in this case is "#app-content" instead of "#content" due to "#app-content" having a background colour set in the server, so if it was set to "#content" the custom background would not be visible. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The slideshow is a direct child of the body that is shown in front of the other elements (header, content and footer) and fills the whole viewport by using absolute positioning. However, as the other elements were still shown behind it they affected the size of the body, and as the body is the scrolling container its scroll bars were shown even if the slideshow was shown. Now the content and the footer are hidden when the slideshow is shown (there is no need to hide the header due to its fixed position, which does not affect the body size), so the body gets the size of the slideshow and thus no scroll bars are shown. Note, however, that when those elements are shown again the body scroll bars will be reset to their initial position, so it is necessary to explicitly restore them to their previous value. This is not needed for the scroll bars of other elements, like the navigation bar or the sidebar, as in that case the whole scrolling container was hidden and shown; in the case of the body the scrolling container is kept and what is hidden and shown are their contents, which alters its size and thus its scroll bars. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
added
bug
Something isn't working
design
Related to the design
regression
Regression of a previous working feature
3. to review
Waiting for reviews
labels
Aug 10, 2018
6 tasks
Or is this ready for review? |
skjnldsv
approved these changes
Aug 30, 2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to take the
Just to clarify: this was a joke based on the message in the OP -> so an actual review happened ;) |
MorrisJobke
approved these changes
Aug 31, 2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and works 👍
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
3. to review
Waiting for reviews
bug
Something isn't working
design
Related to the design
regression
Regression of a previous working feature
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sorry, have to take a bus, can't explain! :-P