[Phpmyadmin-devel] best approach about jQuery dialogs' title bar?
Hi, Tyron Madlener suggested to get rid of the title bar in the create table dialog. I've come up with this patch (done here just for pmahomme): diff --git a/js/functions.js b/js/functions.js index 19ab7ce..d171496 100644 --- a/js/functions.js +++ b/js/functions.js @@ -1514,7 +1514,6 @@ function PMA_createTableDialog( $div, url , target) $div .append(data.error) .dialog({ - title: PMA_messages['strCreateTable'], height: 230, width: 900, open: PMA_verifyColumnsProperties, @@ -1528,7 +1527,7 @@ function PMA_createTableDialog( $div, url , target) $div .append(data) .dialog({ - title: PMA_messages['strCreateTable'], + dialogClass: 'create-table', resizable: false, draggable: false, modal: true, diff --git a/js/messages.php b/js/messages.php index 96f759b..71afc05 100644 --- a/js/messages.php +++ b/js/messages.php @@ -222,7 +222,6 @@ $js_messages['strReloadDatabase'] = __('Reload Database'); $js_messages['strCopyingDatabase'] = __('Copying Database'); $js_messages['strChangingCharset'] = __('Changing Charset'); $js_messages['strTableMustHaveAtleastOneColumn'] = __('Table must have at least one column'); -$js_messages['strCreateTable'] = __('Create Table'); $js_messages['strYes'] = __('Yes'); $js_messages['strNo'] = __('No'); diff --git a/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css b/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css index a712408..a087015 100644 --- a/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css +++ b/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css @@ -437,6 +437,7 @@ button.ui-button::-moz-focus-inner { border: 0; padding: 0; } /* reset extra pad */ .ui-dialog { position: absolute; padding: .2em; width: 300px; overflow: hidden; } .ui-dialog .ui-dialog-titlebar { padding: .4em 1em; position: relative; } +div.create-table .ui-dialog-titlebar {display: none; } .ui-dialog .ui-dialog-title { float: left; margin: .1em 16px .1em 0; } .ui-dialog .ui-dialog-titlebar-close { position: absolute; right: .3em; top: 50%; width: 19px; margin: -10px 0 0 0; padding: 1px; height: 18px; } .ui-dialog .ui-dialog-titlebar-close span { display: block; margin: 1px; } Questions: 1. What do you think of this patch? 2. Should we instead remove the title bar for all our jQuery dialogs? P.S. We'll need to be extra careful when updating the jquery ui, by reinserting phpMyAdmin's customizations under themes. -- Marc Delisle http://infomarc.info
I was just about to check again how it currently looks but the master/ demo seems broken. If I press 'Create table' from the navigation frame in any database the whole content frame turns white after loading. If I press 'create table' again, following error turns up in the console: "Uncaught TypeError: Cannot read property 'scrollWidth' of null" - this happens in chrome 14 / win xp. Also, this Icon is broken (unsharp 200% zoom): http://demo.phpmyadmin.net/master/themes/pmahomme/img/error.ico On Wed, Oct 19, 2011 at 12:26 PM, Marc Delisle <marc@infomarc.info> wrote:
Hi,
Tyron Madlener suggested to get rid of the title bar in the create table dialog.
I've come up with this patch (done here just for pmahomme):
diff --git a/js/functions.js b/js/functions.js index 19ab7ce..d171496 100644 --- a/js/functions.js +++ b/js/functions.js @@ -1514,7 +1514,6 @@ function PMA_createTableDialog( $div, url , target) $div .append(data.error) .dialog({ - title: PMA_messages['strCreateTable'], height: 230, width: 900, open: PMA_verifyColumnsProperties, @@ -1528,7 +1527,7 @@ function PMA_createTableDialog( $div, url , target) $div .append(data) .dialog({ - title: PMA_messages['strCreateTable'], + dialogClass: 'create-table', resizable: false, draggable: false, modal: true, diff --git a/js/messages.php b/js/messages.php index 96f759b..71afc05 100644 --- a/js/messages.php +++ b/js/messages.php @@ -222,7 +222,6 @@ $js_messages['strReloadDatabase'] = __('Reload Database'); $js_messages['strCopyingDatabase'] = __('Copying Database'); $js_messages['strChangingCharset'] = __('Changing Charset'); $js_messages['strTableMustHaveAtleastOneColumn'] = __('Table must have at least one column'); -$js_messages['strCreateTable'] = __('Create Table'); $js_messages['strYes'] = __('Yes'); $js_messages['strNo'] = __('No');
diff --git a/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css b/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css index a712408..a087015 100644 --- a/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css +++ b/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css @@ -437,6 +437,7 @@ button.ui-button::-moz-focus-inner { border: 0; padding: 0; } /* reset extra pad */ .ui-dialog { position: absolute; padding: .2em; width: 300px; overflow: hidden; } .ui-dialog .ui-dialog-titlebar { padding: .4em 1em; position: relative; } +div.create-table .ui-dialog-titlebar {display: none; } .ui-dialog .ui-dialog-title { float: left; margin: .1em 16px .1em 0; } .ui-dialog .ui-dialog-titlebar-close { position: absolute; right: .3em; top: 50%; width: 19px; margin: -10px 0 0 0; padding: 1px; height: 18px; } .ui-dialog .ui-dialog-titlebar-close span { display: block; margin: 1px; }
Questions:
1. What do you think of this patch?
2. Should we instead remove the title bar for all our jQuery dialogs?
P.S. We'll need to be extra careful when updating the jquery ui, by reinserting phpMyAdmin's customizations under themes.
-- Marc Delisle http://infomarc.info
------------------------------------------------------------------------------ All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity and more. Splunk takes this data and makes sense of it. Business sense. IT sense. Common sense. http://p.sf.net/sfu/splunk-d2d-oct _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
2011/10/19 Marc Delisle <marc@infomarc.info>:
Hi,
Tyron Madlener suggested to get rid of the title bar in the create table dialog.
I've come up with this patch (done here just for pmahomme):
diff --git a/js/functions.js b/js/functions.js index 19ab7ce..d171496 100644 --- a/js/functions.js +++ b/js/functions.js @@ -1514,7 +1514,6 @@ function PMA_createTableDialog( $div, url , target) $div .append(data.error) .dialog({ - title: PMA_messages['strCreateTable'], height: 230, width: 900, open: PMA_verifyColumnsProperties, @@ -1528,7 +1527,7 @@ function PMA_createTableDialog( $div, url , target) $div .append(data) .dialog({ - title: PMA_messages['strCreateTable'], + dialogClass: 'create-table', resizable: false, draggable: false, modal: true, diff --git a/js/messages.php b/js/messages.php index 96f759b..71afc05 100644 --- a/js/messages.php +++ b/js/messages.php @@ -222,7 +222,6 @@ $js_messages['strReloadDatabase'] = __('Reload Database'); $js_messages['strCopyingDatabase'] = __('Copying Database'); $js_messages['strChangingCharset'] = __('Changing Charset'); $js_messages['strTableMustHaveAtleastOneColumn'] = __('Table must have at least one column'); -$js_messages['strCreateTable'] = __('Create Table'); $js_messages['strYes'] = __('Yes'); $js_messages['strNo'] = __('No');
diff --git a/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css b/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css index a712408..a087015 100644 --- a/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css +++ b/themes/pmahomme/jquery/jquery-ui-1.8.16.custom.css @@ -437,6 +437,7 @@ button.ui-button::-moz-focus-inner { border: 0; padding: 0; } /* reset extra pad */ .ui-dialog { position: absolute; padding: .2em; width: 300px; overflow: hidden; } .ui-dialog .ui-dialog-titlebar { padding: .4em 1em; position: relative; } +div.create-table .ui-dialog-titlebar {display: none; } .ui-dialog .ui-dialog-title { float: left; margin: .1em 16px .1em 0; } .ui-dialog .ui-dialog-titlebar-close { position: absolute; right: .3em; top: 50%; width: 19px; margin: -10px 0 0 0; padding: 1px; height: 18px; } .ui-dialog .ui-dialog-titlebar-close span { display: block; margin: 1px; }
Questions:
1. What do you think of this patch?
2. Should we instead remove the title bar for all our jQuery dialogs?
P.S. We'll need to be extra careful when updating the jquery ui, by reinserting phpMyAdmin's customizations under themes.
I don't like adding CSS rules to jquery-ui-1.8.16.custom.css. IMO we should do this in theme's CSS or a separate file. Or at the bottom of jquery-ui-1.8.16.custom.css, in a commented section - then future updates of jQuery UI will be simple. Right now it requires to check what changes were done since last update and apply them to new version. -- Piotr Przybylski
Le 2011-10-19 08:53, Piotr Przybylski a écrit :
2011/10/19 Marc Delisle<marc@infomarc.info>:
Hi,
Tyron Madlener suggested to get rid of the title bar in the create table dialog.
I've come up with this patch (done here just for pmahomme): (snip) Questions:
1. What do you think of this patch?
2. Should we instead remove the title bar for all our jQuery dialogs?
P.S. We'll need to be extra careful when updating the jquery ui, by reinserting phpMyAdmin's customizations under themes.
I don't like adding CSS rules to jquery-ui-1.8.16.custom.css. IMO we should do this in theme's CSS or a separate file. Or at the bottom of jquery-ui-1.8.16.custom.css, in a commented section - then future updates of jQuery UI will be simple. Right now it requires to check what changes were done since last update and apply them to new version.
Piotr, about using a separate file, look at commit b857e9580757a84132fc8ccd820a549115af7e2d by Michal, and his comment: "Avoid using overrides for jquery CSS. It is better to modify the style itself instead of including another tiny file with changes." In this commit, Michal removed an override file made by Rouslan in commit 70c70db1392e703346434e65d59110a6ba321367. -- Marc Delisle http://infomarc.info
2011/10/21 Marc Delisle <marc@infomarc.info>:
Le 2011-10-19 08:53, Piotr Przybylski a écrit :
2011/10/19 Marc Delisle<marc@infomarc.info>:
Hi,
Tyron Madlener suggested to get rid of the title bar in the create table dialog.
I've come up with this patch (done here just for pmahomme): (snip) Questions:
1. What do you think of this patch?
2. Should we instead remove the title bar for all our jQuery dialogs?
P.S. We'll need to be extra careful when updating the jquery ui, by reinserting phpMyAdmin's customizations under themes.
I don't like adding CSS rules to jquery-ui-1.8.16.custom.css. IMO we should do this in theme's CSS or a separate file. Or at the bottom of jquery-ui-1.8.16.custom.css, in a commented section - then future updates of jQuery UI will be simple. Right now it requires to check what changes were done since last update and apply them to new version.
Piotr, about using a separate file, look at commit b857e9580757a84132fc8ccd820a549115af7e2d by Michal, and his comment: "Avoid using overrides for jquery CSS. It is better to modify the style itself instead of including another tiny file with changes."
In this commit, Michal removed an override file made by Rouslan in commit 70c70db1392e703346434e65d59110a6ba321367.
Ok, then let's add our styles and overrides in jquery-ui-1.8.16.custom.css, in some commented section at the botom of this file. -- Piotr Przybylski
On Fri, 2011-10-21 at 12:01 +0200, Piotr Przybylski wrote:
2011/10/21 Marc Delisle <marc@infomarc.info>:
Le 2011-10-19 08:53, Piotr Przybylski a écrit :
2011/10/19 Marc Delisle<marc@infomarc.info>:
Hi,
Tyron Madlener suggested to get rid of the title bar in the create table dialog.
I've come up with this patch (done here just for pmahomme): (snip) Questions:
1. What do you think of this patch?
2. Should we instead remove the title bar for all our jQuery dialogs?
P.S. We'll need to be extra careful when updating the jquery ui, by reinserting phpMyAdmin's customizations under themes.
I don't like adding CSS rules to jquery-ui-1.8.16.custom.css. IMO we should do this in theme's CSS or a separate file. Or at the bottom of jquery-ui-1.8.16.custom.css, in a commented section - then future updates of jQuery UI will be simple. Right now it requires to check what changes were done since last update and apply them to new version.
Piotr, about using a separate file, look at commit b857e9580757a84132fc8ccd820a549115af7e2d by Michal, and his comment: "Avoid using overrides for jquery CSS. It is better to modify the style itself instead of including another tiny file with changes."
In this commit, Michal removed an override file made by Rouslan in commit 70c70db1392e703346434e65d59110a6ba321367.
Ok, then let's add our styles and overrides in jquery-ui-1.8.16.custom.css, in some commented section at the botom of this file.
If the problem is just the extra http request for a tiny override file, then why don't we just concatenate the two css files dynamically? E.g.: <?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); require 'jquery-ui-1.8.16.custom.css'; echo "\n"; @include 'jquery-ui-1.8.16.overrides.css'; ?> Rouslan
2011/10/21 Rouslan Placella <rouslan@placella.com>:
On Fri, 2011-10-21 at 12:01 +0200, Piotr Przybylski wrote:
2011/10/21 Marc Delisle <marc@infomarc.info>:
Le 2011-10-19 08:53, Piotr Przybylski a écrit :
2011/10/19 Marc Delisle<marc@infomarc.info>:
Hi,
Tyron Madlener suggested to get rid of the title bar in the create table dialog.
I've come up with this patch (done here just for pmahomme): (snip) Questions:
1. What do you think of this patch?
2. Should we instead remove the title bar for all our jQuery dialogs?
P.S. We'll need to be extra careful when updating the jquery ui, by reinserting phpMyAdmin's customizations under themes.
I don't like adding CSS rules to jquery-ui-1.8.16.custom.css. IMO we should do this in theme's CSS or a separate file. Or at the bottom of jquery-ui-1.8.16.custom.css, in a commented section - then future updates of jQuery UI will be simple. Right now it requires to check what changes were done since last update and apply them to new version.
Piotr, about using a separate file, look at commit b857e9580757a84132fc8ccd820a549115af7e2d by Michal, and his comment: "Avoid using overrides for jquery CSS. It is better to modify the style itself instead of including another tiny file with changes."
In this commit, Michal removed an override file made by Rouslan in commit 70c70db1392e703346434e65d59110a6ba321367.
Ok, then let's add our styles and overrides in jquery-ui-1.8.16.custom.css, in some commented section at the botom of this file.
If the problem is just the extra http request for a tiny override file, then why don't we just concatenate the two css files dynamically? E.g.:
<?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); require 'jquery-ui-1.8.16.custom.css'; echo "\n"; @include 'jquery-ui-1.8.16.overrides.css'; ?>
It doesn't concatenate, it just adds CSS @include, which will fire a http request. It's better to 'include' override in phpmyadmin.css, which already serves our CSS. -- Piotr Przybylski
On Fri, 2011-10-21 at 12:21 +0200, Piotr Przybylski wrote:
2011/10/21 Rouslan Placella <rouslan@placella.com>:
On Fri, 2011-10-21 at 12:01 +0200, Piotr Przybylski wrote:
2011/10/21 Marc Delisle <marc@infomarc.info>:
Le 2011-10-19 08:53, Piotr Przybylski a écrit :
2011/10/19 Marc Delisle<marc@infomarc.info>:
Hi,
Tyron Madlener suggested to get rid of the title bar in the create table dialog.
I've come up with this patch (done here just for pmahomme): (snip) Questions:
1. What do you think of this patch?
2. Should we instead remove the title bar for all our jQuery dialogs?
P.S. We'll need to be extra careful when updating the jquery ui, by reinserting phpMyAdmin's customizations under themes.
I don't like adding CSS rules to jquery-ui-1.8.16.custom.css. IMO we should do this in theme's CSS or a separate file. Or at the bottom of jquery-ui-1.8.16.custom.css, in a commented section - then future updates of jQuery UI will be simple. Right now it requires to check what changes were done since last update and apply them to new version.
Piotr, about using a separate file, look at commit b857e9580757a84132fc8ccd820a549115af7e2d by Michal, and his comment: "Avoid using overrides for jquery CSS. It is better to modify the style itself instead of including another tiny file with changes."
In this commit, Michal removed an override file made by Rouslan in commit 70c70db1392e703346434e65d59110a6ba321367.
Ok, then let's add our styles and overrides in jquery-ui-1.8.16.custom.css, in some commented section at the botom of this file.
If the problem is just the extra http request for a tiny override file, then why don't we just concatenate the two css files dynamically? E.g.:
<?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); require 'jquery-ui-1.8.16.custom.css'; echo "\n"; @include 'jquery-ui-1.8.16.overrides.css'; ?>
It doesn't concatenate, it just adds CSS @include, which will fire a http request. It's better to 'include' override in phpmyadmin.css, which already serves our CSS.
You're thinking about the CSS @import rule, which will fire the extra request. Let me re-write the above snippet (same functionality): <?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); echo file_get_contents('jquery-ui-1.8.16.custom.css') . "\n"; $override = 'jquery-ui-1.8.16.overrides.css'; if (is_readable($override)) { echo file_get_contents($override); } ?>
Le 2011-10-21 06:31, Rouslan Placella a écrit :
On Fri, 2011-10-21 at 12:21 +0200, Piotr Przybylski wrote:
2011/10/21 Rouslan Placella<rouslan@placella.com>:
On Fri, 2011-10-21 at 12:01 +0200, Piotr Przybylski wrote:
2011/10/21 Marc Delisle<marc@infomarc.info>:
Le 2011-10-19 08:53, Piotr Przybylski a écrit :
2011/10/19 Marc Delisle<marc@infomarc.info>: > Hi, > > Tyron Madlener suggested to get rid of the title bar in the create table > dialog. > > I've come up with this patch (done here just for pmahomme): (snip) > Questions: > > 1. What do you think of this patch? > > 2. Should we instead remove the title bar for all our jQuery dialogs? > > P.S. We'll need to be extra careful when updating the jquery ui, by > reinserting phpMyAdmin's customizations under themes.
I don't like adding CSS rules to jquery-ui-1.8.16.custom.css. IMO we should do this in theme's CSS or a separate file. Or at the bottom of jquery-ui-1.8.16.custom.css, in a commented section - then future updates of jQuery UI will be simple. Right now it requires to check what changes were done since last update and apply them to new version.
Piotr, about using a separate file, look at commit b857e9580757a84132fc8ccd820a549115af7e2d by Michal, and his comment: "Avoid using overrides for jquery CSS. It is better to modify the style itself instead of including another tiny file with changes."
In this commit, Michal removed an override file made by Rouslan in commit 70c70db1392e703346434e65d59110a6ba321367.
Ok, then let's add our styles and overrides in jquery-ui-1.8.16.custom.css, in some commented section at the botom of this file.
If the problem is just the extra http request for a tiny override file, then why don't we just concatenate the two css files dynamically? E.g.:
<?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); require 'jquery-ui-1.8.16.custom.css'; echo "\n"; @include 'jquery-ui-1.8.16.overrides.css'; ?>
It doesn't concatenate, it just adds CSS @include, which will fire a http request. It's better to 'include' override in phpmyadmin.css, which already serves our CSS.
You're thinking about the CSS @import rule, which will fire the extra request. Let me re-write the above snippet (same functionality):
<?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); echo file_get_contents('jquery-ui-1.8.16.custom.css') . "\n"; $override = 'jquery-ui-1.8.16.overrides.css'; if (is_readable($override)) { echo file_get_contents($override); } ?>
This makes sense. -- Marc Delisle http://infomarc.info
On Sun, 2011-10-23 at 09:46 -0400, Marc Delisle wrote:
Le 2011-10-21 06:31, Rouslan Placella a écrit :
On Fri, 2011-10-21 at 12:21 +0200, Piotr Przybylski wrote:
2011/10/21 Rouslan Placella<rouslan@placella.com>:
On Fri, 2011-10-21 at 12:01 +0200, Piotr Przybylski wrote:
2011/10/21 Marc Delisle<marc@infomarc.info>:
Le 2011-10-19 08:53, Piotr Przybylski a écrit : > 2011/10/19 Marc Delisle<marc@infomarc.info>: >> Hi, >> >> Tyron Madlener suggested to get rid of the title bar in the create table >> dialog. >> >> I've come up with this patch (done here just for pmahomme): (snip) >> Questions: >> >> 1. What do you think of this patch? >> >> 2. Should we instead remove the title bar for all our jQuery dialogs? >> >> P.S. We'll need to be extra careful when updating the jquery ui, by >> reinserting phpMyAdmin's customizations under themes. > > I don't like adding CSS rules to jquery-ui-1.8.16.custom.css. IMO we > should do this in theme's CSS or a separate file. Or at the bottom of > jquery-ui-1.8.16.custom.css, in a commented section - then future > updates of jQuery UI will be simple. Right now it requires to check > what changes were done since last update and apply them to new > version. >
Piotr, about using a separate file, look at commit b857e9580757a84132fc8ccd820a549115af7e2d by Michal, and his comment: "Avoid using overrides for jquery CSS. It is better to modify the style itself instead of including another tiny file with changes."
In this commit, Michal removed an override file made by Rouslan in commit 70c70db1392e703346434e65d59110a6ba321367.
Ok, then let's add our styles and overrides in jquery-ui-1.8.16.custom.css, in some commented section at the botom of this file.
If the problem is just the extra http request for a tiny override file, then why don't we just concatenate the two css files dynamically? E.g.:
<?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); require 'jquery-ui-1.8.16.custom.css'; echo "\n"; @include 'jquery-ui-1.8.16.overrides.css'; ?>
It doesn't concatenate, it just adds CSS @include, which will fire a http request. It's better to 'include' override in phpmyadmin.css, which already serves our CSS.
You're thinking about the CSS @import rule, which will fire the extra request. Let me re-write the above snippet (same functionality):
<?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); echo file_get_contents('jquery-ui-1.8.16.custom.css') . "\n"; $override = 'jquery-ui-1.8.16.overrides.css'; if (is_readable($override)) { echo file_get_contents($override); } ?>
This makes sense.
Then how about factoring out the 'jquery-ui-1.8.16.custom.css' file and placing it into the 'themes' folder at top level and having overrides in each theme (if any)?
Le 2011-10-23 14:39, Rouslan Placella a écrit :
On Sun, 2011-10-23 at 09:46 -0400, Marc Delisle wrote:
Le 2011-10-21 06:31, Rouslan Placella a écrit :
On Fri, 2011-10-21 at 12:21 +0200, Piotr Przybylski wrote:
2011/10/21 Rouslan Placella<rouslan@placella.com>:
On Fri, 2011-10-21 at 12:01 +0200, Piotr Przybylski wrote:
2011/10/21 Marc Delisle<marc@infomarc.info>: > Le 2011-10-19 08:53, Piotr Przybylski a écrit : >> 2011/10/19 Marc Delisle<marc@infomarc.info>: >>> Hi, >>> >>> Tyron Madlener suggested to get rid of the title bar in the create table >>> dialog. >>> >>> I've come up with this patch (done here just for pmahomme): > (snip) >>> Questions: >>> >>> 1. What do you think of this patch? >>> >>> 2. Should we instead remove the title bar for all our jQuery dialogs? >>> >>> P.S. We'll need to be extra careful when updating the jquery ui, by >>> reinserting phpMyAdmin's customizations under themes. >> >> I don't like adding CSS rules to jquery-ui-1.8.16.custom.css. IMO we >> should do this in theme's CSS or a separate file. Or at the bottom of >> jquery-ui-1.8.16.custom.css, in a commented section - then future >> updates of jQuery UI will be simple. Right now it requires to check >> what changes were done since last update and apply them to new >> version. >> > > Piotr, > about using a separate file, look at commit > b857e9580757a84132fc8ccd820a549115af7e2d by Michal, and his comment: > "Avoid using overrides for jquery CSS. It is better to modify the style > itself instead of including another tiny file with changes." > > In this commit, Michal removed an override file made by Rouslan in > commit 70c70db1392e703346434e65d59110a6ba321367. >
Ok, then let's add our styles and overrides in jquery-ui-1.8.16.custom.css, in some commented section at the botom of this file.
If the problem is just the extra http request for a tiny override file, then why don't we just concatenate the two css files dynamically? E.g.:
<?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); require 'jquery-ui-1.8.16.custom.css'; echo "\n"; @include 'jquery-ui-1.8.16.overrides.css'; ?>
It doesn't concatenate, it just adds CSS @include, which will fire a http request. It's better to 'include' override in phpmyadmin.css, which already serves our CSS.
You're thinking about the CSS @import rule, which will fire the extra request. Let me re-write the above snippet (same functionality):
<?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); echo file_get_contents('jquery-ui-1.8.16.custom.css') . "\n"; $override = 'jquery-ui-1.8.16.overrides.css'; if (is_readable($override)) { echo file_get_contents($override); } ?>
This makes sense.
Then how about factoring out the 'jquery-ui-1.8.16.custom.css' file and placing it into the 'themes' folder at top level and having overrides in each theme (if any)?
Fine by me. -- Marc Delisle http://infomarc.info
On Sun, 2011-10-23 at 11:41 -0400, Marc Delisle wrote:
Le 2011-10-23 14:39, Rouslan Placella a écrit :
On Sun, 2011-10-23 at 09:46 -0400, Marc Delisle wrote:
Le 2011-10-21 06:31, Rouslan Placella a écrit :
On Fri, 2011-10-21 at 12:21 +0200, Piotr Przybylski wrote:
2011/10/21 Rouslan Placella<rouslan@placella.com>:
On Fri, 2011-10-21 at 12:01 +0200, Piotr Przybylski wrote: > 2011/10/21 Marc Delisle<marc@infomarc.info>: >> Le 2011-10-19 08:53, Piotr Przybylski a écrit : >>> 2011/10/19 Marc Delisle<marc@infomarc.info>: >>>> Hi, >>>> >>>> Tyron Madlener suggested to get rid of the title bar in the create table >>>> dialog. >>>> >>>> I've come up with this patch (done here just for pmahomme): >> (snip) >>>> Questions: >>>> >>>> 1. What do you think of this patch? >>>> >>>> 2. Should we instead remove the title bar for all our jQuery dialogs? >>>> >>>> P.S. We'll need to be extra careful when updating the jquery ui, by >>>> reinserting phpMyAdmin's customizations under themes. >>> >>> I don't like adding CSS rules to jquery-ui-1.8.16.custom.css. IMO we >>> should do this in theme's CSS or a separate file. Or at the bottom of >>> jquery-ui-1.8.16.custom.css, in a commented section - then future >>> updates of jQuery UI will be simple. Right now it requires to check >>> what changes were done since last update and apply them to new >>> version. >>> >> >> Piotr, >> about using a separate file, look at commit >> b857e9580757a84132fc8ccd820a549115af7e2d by Michal, and his comment: >> "Avoid using overrides for jquery CSS. It is better to modify the style >> itself instead of including another tiny file with changes." >> >> In this commit, Michal removed an override file made by Rouslan in >> commit 70c70db1392e703346434e65d59110a6ba321367. >> > > Ok, then let's add our styles and overrides in > jquery-ui-1.8.16.custom.css, in some commented section at the botom of > this file. >
If the problem is just the extra http request for a tiny override file, then why don't we just concatenate the two css files dynamically? E.g.:
<?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); require 'jquery-ui-1.8.16.custom.css'; echo "\n"; @include 'jquery-ui-1.8.16.overrides.css'; ?>
It doesn't concatenate, it just adds CSS @include, which will fire a http request. It's better to 'include' override in phpmyadmin.css, which already serves our CSS.
You're thinking about the CSS @import rule, which will fire the extra request. Let me re-write the above snippet (same functionality):
<?php // file: jquery-ui.css.php header('Content-Type: text/css; charset=UTF-8'); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600) . ' GMT'); echo file_get_contents('jquery-ui-1.8.16.custom.css') . "\n"; $override = 'jquery-ui-1.8.16.overrides.css'; if (is_readable($override)) { echo file_get_contents($override); } ?>
This makes sense.
Then how about factoring out the 'jquery-ui-1.8.16.custom.css' file and placing it into the 'themes' folder at top level and having overrides in each theme (if any)?
Fine by me.
I had a go at implementing this, but it doesn't actually work. The problem is that by moving jquery-ui-1.8.16.custom.css to the top level of the themes folder the relative links to images that are inside it break. But we would definitely would want themers to be able to customise these. Rouslan
Hi Dne Sun, 23 Oct 2011 19:39:04 +0100 Rouslan Placella <rouslan@placella.com> napsal(a):
Then how about factoring out the 'jquery-ui-1.8.16.custom.css' file and placing it into the 'themes' folder at top level and having overrides in each theme (if any)?
Sounds good. -- Michal Čihař | http://cihar.com | http://phpmyadmin.cz
Hi Dne Fri, 21 Oct 2011 05:52:10 -0400 Marc Delisle <marc@infomarc.info> napsal(a):
about using a separate file, look at commit b857e9580757a84132fc8ccd820a549115af7e2d by Michal, and his comment: "Avoid using overrides for jquery CSS. It is better to modify the style itself instead of including another tiny file with changes."
In this commit, Michal removed an override file made by Rouslan in commit 70c70db1392e703346434e65d59110a6ba321367.
The problem there was using separate file, later the style changes were AFAIK moved to theme CSS keeping jquery CSS intact. -- Michal Čihař | http://cihar.com | http://phpmyadmin.cz
Le 2011-10-21 09:09, Michal Čihař a écrit :
Hi
Dne Fri, 21 Oct 2011 05:52:10 -0400 Marc Delisle<marc@infomarc.info> napsal(a):
about using a separate file, look at commit b857e9580757a84132fc8ccd820a549115af7e2d by Michal, and his comment: "Avoid using overrides for jquery CSS. It is better to modify the style itself instead of including another tiny file with changes."
In this commit, Michal removed an override file made by Rouslan in commit 70c70db1392e703346434e65d59110a6ba321367.
The problem there was using separate file, later the style changes were AFAIK moved to theme CSS keeping jquery CSS intact.
In master, there are currently two copies of jquery-ui-1.8.16.custom.css, one under each theme/theme_name/jquery. Moreover, these copies are identical. -- Marc Delisle http://infomarc.info
participants (5)
-
Marc Delisle -
Michal Čihař -
Piotr Przybylski -
Rouslan Placella -
Tyron Madlener