View Issue Details

IDProjectCategoryView StatusLast Update
0020224mantisbtcode cleanuppublic2016-10-20 08:05
Reporterdregad Assigned Tocproensa  
PrioritynormalSeverityminorReproducibilityN/A
Status closedResolutionwon't fix 
Summary0020224: Refactor collapse API
Description

As reported by cproensa in 0019508:0051686, the collapse API's suffers from design issues, e.g.

  • 2 separate DIVs are created (open and closed), which are alternatively shown/hidden via javascript depending on collapse state. When JS is disabled, both divs are shown (this is the issue originally reported in 0019508)
  • when JS is enabled, the search field in the filter section is actually 2 distinct text fields, so any criteria entered will no longer be available when the section is collapsed/expanded

It should be refactored to better handle these issues.

Additional Information

cproensa's suggestions for improvements:

  • [0019508:0051699] A more sane way to do the open/collapse would be by toggling the visibility of the actual - contents, not by maintaining two different section containers.

[div "frame"]
[div "inner"]
...content...
[/div]
[/div]

inner.hide() / inner.show()

  • [0019508:0051704] A show/hide logic for the collapsing system should be driven by classes, and content shlould also be unique.
    In the sponshorsip case (the title changes when it's collapsed to show the total amount), at first sight looks like its unnecessary to have that differentiation on title ¿why not show all the info, always?
    .. and if really needed, type it with meaningful classes.
    Eg: all ".collapsed-hide" elements will be hidden, be it the iner div, or a [span] part in the title.
    You could even have a ".collapsed-show" class for content that should be visible only when the section is collapsed
TagsNo tags attached.
Attached Files

Relationships

related to 0019508 closeddregad Regression when using MantisBT in a browser where JavaScript is disabled 
related to 0005936 closedcproensa Search resets filter if filter-panel collapsed 

Activities

cproensa

cproensa

2015-10-25 07:03

developer   ~0051717

what timeframe do you suggest?

i guess this would imply:

  • create new functions, css classes,
  • modify all callers of collapse_api (the duplication of content is made at the caller, with direct output that cannot be controlled by the api itslef)
  • deprecate previous functionality, and keep it alive in case some plugins are using it
dregad

dregad

2015-10-25 14:50

developer   ~0051719

Given my lack of available time I am not suggesting any timeframe ;-)
If you or someone else has the bandwidth to propose something, then I think it's worth targeting 1.3.0, although it's not a blocker if an acceptable workaround can be found for the duplicated sections issue (0019508).

Your proposed plan is more or less what I had in mind.

cproensa

cproensa

2015-10-25 15:22

developer   ~0051720

i am no expert in css stuff, but this is somehow what i had in mind:
https://jsfiddle.net/r8kxypw3/1/

with that scheme, all the html could be done even without any call to a collapse_api.
However, one function to get current state of each section (remembering the user settings), is needed, the same way its is done now with cookies (no need to change that)

This can be retrofitted to the section tables like those in bug_view. altough, the ideal goal is to use table-less sections (there already exists the section-container class...)

cproensa

cproensa

2015-10-25 15:25

developer   ~0051721

the updated skecth is this one, srry:
http://jsfiddle.net/r8kxypw3/2/

cproensa

cproensa

2015-10-26 09:05

developer   ~0051727

I you can live without the alternating contents in header, and keep a strict structure, an almost transparent approach can be presented:
http://jsfiddle.net/r8kxypw3/17/

This, however, requires rewrite the section as div containers (which may be a good point anyway)
The problem is current sections are tables, being the section header the first row, and the presented functionality cant be fitted into it, because its not valid to place div elements interleaved with table structure (outside of TD blocks)

dregad

dregad

2015-10-27 04:12

developer   ~0051731

Thanks for your research.

The problem is current sections are tables

This is a legacy from <= 1.2, and one of the things that need to be changed; a lot of this kind of cleanup has already been implemented in 1.3... Turning these into divs and only keeping html tables to display actual tabular data within the divs, would be the right thing to do IMO, even though it requires additional effort.

http://jsfiddle.net/r8kxypw3/2/
http://jsfiddle.net/r8kxypw3/17/

Both look like nice solutions, v17 seems cleaner.

I you can live without the alternating contents in header

Need to check how frequently this is actually used, and how important that actually is; if it's only Sponsorship, I'm pretty sure we can live without it.

without any call to a collapse_api

Relying on an API has the advantage of ensuring consistency across the board, by abstracting the details of containers, classes, etc.

cproensa

cproensa

2015-10-27 19:01

developer   ~0051741

Relying on an API has the advantage of ensuring consistency across the board, by abstracting the details of containers, classes, etc.

of course, thats important.
however, if the functionality is transparent, there's not any additional call needed to be made to generate the html code. Consistency is then achieved by the html itself, just like as how its done now for laying a form, or tables, with current mantis css standards, using the established tags to achieve the correct formatting.

attached is a patch to test the concept:
0001-adding-collapseconcpet_adaptview_user_page.php.patch
(this is more or less the jsfiddle/17)

included in teh patch there is a rewrite of "view_user_page.php", which i think its an existing example of a section layout.

As for the actual HTML, the only modification done in that page is:
from:

 <div class="section-container"> 

to:
 <div class="section-container collapse"> 

and ultimately, using a function to recall user preference:
 <div class="section-container collapse <?php echo collapse_get_default_state()?>"> 

I tried to get this one to be simple and consistent with current formatting, and for the moment i see it nice.old code can be

so enough with this, keep it as a possiblity.
Let other ideas flow... come on!

0001-adding-collapseconcpet_adaptview_user_page.php.patch (2,181 bytes)   
From c24c1d8e3f9bbfca6969e16de86a8088e5a611f7 Mon Sep 17 00:00:00 2001
From: Carlos Proensa <proensa@gmail.com>
Date: Tue, 27 Oct 2015 23:25:35 +0100
Subject: [PATCH] adding collapse concpet & adapt view_user_page.php

---
 css/default.css      | 6 ++++++
 javascript/common.js | 8 ++++++++
 view_user_page.php   | 2 +-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/css/default.css b/css/default.css
index 48f219c..3d6377d 100644
--- a/css/default.css
+++ b/css/default.css
@@ -804,3 +804,9 @@ div.timeline * div.action { padding-top: 3px; }
 /* bug_view_inc bug action buttons */
 .details-footer      { padding: 3px 10px 2px 0; }
 .details-buttons     { float: left;  width: auto; }
+
+.js .collapse > h2:before { padding-right: 5px; content: url("../images/minus.png"); }
+.js .collapse.collapsed > h2:before {  padding-right: 5px; content: url("../images/plus.png"); }
+.js .collapse.collapsed > *:not(h2) { display:none; }
+.js div.section-container.collapse { display: block; }
+.js div.section-container.collapse > h2 { float: none; }
\ No newline at end of file
diff --git a/javascript/common.js b/javascript/common.js
index 34c1f91..c842003 100644
--- a/javascript/common.js
+++ b/javascript/common.js
@@ -35,6 +35,14 @@ if (a!= -1) {
 style_display = 'block';
 
 $(document).ready( function() {
+	document.documentElement.className += " js "
+    $('.collapse > h2').click(function (event) {
+        $(this).parent('.collapse').toggleClass('collapsed');
+    });
+    $('.collapse > h2 > *').click(function (event) {
+        event.stopPropagation();
+    });
+
 	$('.collapse-open').show();
 	$('.collapse-closed').hide();
 	$('.collapse-link').click( function(event) {
diff --git a/view_user_page.php b/view_user_page.php
index 690e1f9..30c0328 100644
--- a/view_user_page.php
+++ b/view_user_page.php
@@ -74,7 +74,7 @@ $u_realname = user_get_realname( $u_id );
 html_page_top();
 ?>
 
-<div class="section-container">
+<div class="section-container collapse">
 	<h2><?php echo lang_get( 'view_account_title' ) ?></h2>
 	<div class="field-container">
 		<span class="display-label"><span><?php echo lang_get( 'username' ) ?></span></span>
-- 
1.9.1

cproensa

cproensa

2016-10-20 08:05

developer   ~0054304

As this refers to v1.3, and this api has been already refactored for v2, i'm closing this as no change.