Patch #30448
closed
Remove wrapper2 and wrapper3 wrapping containers
0%
Description
I think maybe it would be good to remove the two wrappers (wrapper2 and wrapper3), as there is really no purpose in them, other than making the need to add specific extra rules just for these containers, because children containers often behave depending on the rules set to their parent. And so, wrappers can spawn a blocking behaviour for the rules applied to their far inner children in one way or another.
I think it will be good to leave only the top wrapper (at least for now) which would wrap everything except for the footer container, this is to achieve sticky footer design later which will be more easily achievable like this, or else footer winds up in the middle of the screen when there is little to no content on pages (I have quite a big display). The method I'm suggesting is to stretch the html, body and wrapper 100% in height, by setting height:100% rule to them, and then apply margin-top: -25px to both the footer and margin-bottom: -25px to the wrapper.
Then #content containers also will not need min-height: 600px rule in the future.
Files
Related issues
Updated by Marius BĂLTEANU almost 6 years ago
- Related to Patch #30231: Operation: "Unwraping The Mine" or conerns about how elements are wrapped added
Updated by Anonymous almost 6 years ago
Update: while developing quick html concept layouts for #30451 (#30451#note-3 and #30451#note-5), I came to conclusion that maybe we don't need even the first #wrapper as well, things could just be rerouted to body tag and sticky footer feature is also achievable without the wrapper.
Updated by Jean-Philippe Lang over 5 years ago
- Target version changed from 4.1.0 to 5.0.0
We remove tags from the layout and some themes may rely on them.
Better set it to a major release.
Updated by Anonymous over 5 years ago
Actively maintained themes have to be anyway patched on per minor release basis, and not actively maintained themes shouldn't be our responsibility really. I bet those wrappers aren't even the most important things that may affect things in themes, as even minor releases proven to break a lot of things already. There is really no point in waiting until 5.0
By the time we ll get to 5.0, the world will move on once again.
Updated by Marius BĂLTEANU over 3 years ago
- Assignee changed from Jean-Philippe Lang to Marius BĂLTEANU
Updated by Marius BĂLTEANU over 3 years ago
- File 0001-Remove-wrapper2-and-wrapper3-wrapping-containers-304.patch 0001-Remove-wrapper2-and-wrapper3-wrapping-containers-304.patch added
Updated the patch.
Updated by Mizuki ISHIKAWA about 3 years ago
- File hamburger-menu.png hamburger-menu.png added
After this change, the hamburger menu links that appear when the screen width is narrow no longer work.
Clicking on the Overview, Activity, etc. links in the next image will close the menu without taking you anywhere.
With the change in https://www.redmine.org/projects/redmine/repository/revisions/21151/diff/trunk/public/javascripts/responsive.js, the timing for closing the menu has been changed from $('#wrapper2').on('click', ... to $('#wrapper').on('click', ... .
The #wrapper2 was an element that did not contain the hamburger menu element(.flyout-menu), but the #wrapper contains the .flyout-menu.
This means that when you click on a link in the menu, it will be judged that you have clicked on the #wrapper, and the process of closing the menu will be running.
Updated by Marius BĂLTEANU about 3 years ago
- Status changed from Closed to Reopened
Updated by Mizuki ISHIKAWA about 3 years ago
I think the following changes will solve the problem described in #30448#note-9.
The problem only occurs with trunk.
diff --git a/public/javascripts/responsive.js b/public/javascripts/responsive.js
index 0bc5c3452a..ddfaee6fb8 100644
--- a/public/javascripts/responsive.js
+++ b/public/javascripts/responsive.js
@@ -3,9 +3,11 @@
function openFlyout() {
$('html').addClass('flyout-is-active');
$('#wrapper').on('click', function(e){
- e.preventDefault();
- e.stopPropagation();
- closeFlyout();
+ if (!$(e.target).closest('.flyout-menu').length) {
+ e.preventDefault();
+ e.stopPropagation();
+ closeFlyout();
+ }
});
}
Updated by Bernhard Rohloff about 3 years ago
I've tested Mizuki's patch right now and it works perfectly.
Additionally, I found a legacy of #wrapper2 in the responsive.css.
Index: public/stylesheets/responsive.css
===================================================================
--- public/stylesheets/responsive.css (Revision 21256)
+++ public/stylesheets/responsive.css (Arbeitskopie)
@@ -553,7 +553,7 @@
content: '\00D7'; /* close glyph */
}
- .flyout-is-active #wrapper2 {
+ .flyout-is-active #main {
/*
* only relevant for devices with cursor when flyout it active, in order to show,
* that whole wrapper content is clickable and closes flyout menu
Updated by Marius BĂLTEANU about 3 years ago
Mizuki ISHIKAWA wrote:
I think the following changes will solve the problem described in #30448#note-9.
The problem only occurs with trunk.[...]
What do you think about this fix?
diff --git a/public/javascripts/responsive.js b/public/javascripts/responsive.js
index 0bc5c3452..a07737795 100644
--- a/public/javascripts/responsive.js
+++ b/public/javascripts/responsive.js
@@ -2,7 +2,7 @@
function openFlyout() {
$('html').addClass('flyout-is-active');
- $('#wrapper').on('click', function(e){
+ $('#main').on('click', function(e){
e.preventDefault();
e.stopPropagation();
closeFlyout();
Updated by Mizuki ISHIKAWA about 3 years ago
Marius BALTEANU wrote:
What do you think about this fix?
[...]
It looks good.
Changing it to #main will prevent the menu from closing when you click on the #fotter. However, since the #footer is small, this should not be a problem.
Updated by Marius BĂLTEANU about 3 years ago
- Status changed from Reopened to Closed
Fix committed, thanks for catching this issue.
Updated by Go MAEDA 12 months ago
- Related to Defect #39802: Fix click event handling in mobile view after closing flyout menu added