Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[Contest-6] No hard-coded icons in css #200

Merged
merged 5 commits into from

4 participants

@hisamith

All of the hard-coded images that are also available as a css sprite replaced by their sprite counterparts.
Remove unused css styles for iconic view for ul items.
Update the css file in the "original" theme.

hisamith added some commits
@hisamith hisamith [Contest1] No hard-coded icons in css
Modified "list-style-type" to "none" for the list items that has an image as the bullet point and aligned them
c88420a
@hisamith hisamith All of the hard-coded images that are also available as a CSS sprite …
…replaced by their sprite counterparts.

Remove unused css styles for iconic view for ul items.
Update the CSS file in the "original" theme.
bc47ff4
@nijel
Owner

Please fix the testsuite so that it does not fail with your changes.

@hisamith

Hi Michal,
I have updated the test cases, according to the modifications that I have done

libraries/Message.class.php
@@ -651,7 +654,7 @@ public function getMessage()
return $message;
}
-
+
@ruleant
ruleant added a note

try to avoid space characters on empty lines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ruleant

a few general remarks:

  • our coding style defines not to use lines longer than 85 characters. You can use phpcs (PHP Code Sniffer, see http://wiki.phpmyadmin.net/pma/Jenkins_Setup to install) to check violations against the codingstyle we use
  • when concatenating with . leave a space before and after the dot. This makes the code more readable
  • watch out for space characters on empty lines and trailing spaces
@ruleant

Coding style looks good, thanks!

@ruleant

Tested and it seems to work.
I noticed that some sections where removed from the css files, (fe. common.css.php lines 1284-1302, 1308-1362, fe. li_mysql_collations) but no code was added to insert this icon in the appropriate place.

Can someone else confirm that the right icons are already in the code? @roccivic, @lem9, @nijel?

@lem9
Owner

Dieter,
do you mean li_select_mysql_collation ? I see that some code has been inserted by hisamith in index.php to call
PMA_Util::getImage('s_asci.png').

@ruleant

No, for the removed lines in common.css.php 1654-1680, you see a replacement in index.php, but the lines in common.css.php I mentioned in the previous comment, I don't see a replacement in other code. That's why I was wondering if there should be a replacement in the code, or maybe it is already there and the icon's in the css are obsolete already.
Looking for those items in the code (I tried 'grep -r li_select_mysql_collation *') doesn't turn up a match, so that's why I'm wondering if they are still needed.

@lem9
Owner

Dieter,
let's take an example. You mention lines 1284-1302 (I believe you refer to bc47ff4). On line 1285, we see he removed the reference to b_newdb.png. However, in c88420a we see that he added a reference to this icon in libraries/display_create_database.lib.php.

@ruleant

The lines 1284-1302 are OK, the icons were removed from the css and inserted to the appropriate places in the code. I referenced the wrong range (1284-1302) in a previous comment, my bad.

I mean the lines 1308-1362 (1678-1732 in the pmahomme version) : 'li_mysql_variables' to 'li_user_preferences'
A quick look for some of the icons (*.png) in the current code base, turns up they are used, so I guess their occurence in the css files is obsolete and they can be removed.
But because I didn't see any replacement in hisamith's patch, I was wondering if they were forgotten, or were obsolete and could be removed.

@lem9
Owner

Yes, they must be obsolete.

@lem9 lem9 merged commit 79b7d3c into phpmyadmin:master
@lem9
Owner

We have a winner for Contest 6, congratulations.

@hisamith

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 2, 2013
  1. @hisamith

    [Contest1] No hard-coded icons in css

    hisamith authored
    Modified "list-style-type" to "none" for the list items that has an image as the bullet point and aligned them
Commits on Mar 5, 2013
  1. @hisamith

    All of the hard-coded images that are also available as a CSS sprite …

    hisamith authored
    …replaced by their sprite counterparts.
    
    Remove unused css styles for iconic view for ul items.
    Update the CSS file in the "original" theme.
Commits on Mar 6, 2013
  1. @hisamith
Commits on Mar 8, 2013
  1. @hisamith

    Fixed coding style

    hisamith authored
  2. @hisamith

    Fixed coding style

    hisamith authored
This page is out of date. Refresh to see the latest.
View
30 index.php
@@ -120,9 +120,9 @@
if (! $cfg['NavigationDisplayServers']
&& (count($cfg['Servers']) > 1 || $server == 0 && count($cfg['Servers']) == 1)
) {
- echo '<li id="li_select_server">';
+ echo '<li id="li_select_server" class="no_bullets" >';
include_once 'libraries/select_server.lib.php';
- echo PMA_selectServer(true, true);
+ echo PMA_Util::getImage('s_host.png') . " " . PMA_selectServer(true, true);
echo '</li>';
}
@@ -137,22 +137,23 @@
if ($cfg['ShowChgPassword']) {
$conditional_class = 'ajax';
PMA_printListItem(
- __('Change password'),
+ PMA_Util::getImage('s_passwd.png') . " " . __('Change password'),
'li_change_password',
'user_password.php?' . $common_url_query,
null,
null,
'change_password_anchor',
- null,
+ "no_bullets",
$conditional_class
);
}
} // end if
- echo ' <li id="li_select_mysql_collation">';
+ echo ' <li id="li_select_mysql_collation" class="no_bullets" >';
echo ' <form method="post" action="index.php">' . "\n"
. PMA_generate_common_hidden_inputs(null, null, 4, 'collation_connection')
. ' <label for="select_collation_connection">' . "\n"
- . ' ' . __('Server connection collation') . "\n"
+ . ' '. PMA_Util::getImage('s_asci.png') . " "
+ . __('Server connection collation') . "\n"
// put the doc link in the form so that it appears on the same line
. PMA_Util::showMySQLDocu(
'MySQL_Database_Administration',
@@ -183,17 +184,18 @@
// Displays language selection combo
if (empty($cfg['Lang']) && count($GLOBALS['available_languages']) > 1) {
- echo '<li id="li_select_lang">';
+ echo '<li id="li_select_lang" class="no_bullets">';
include_once 'libraries/display_select_lang.lib.php';
- echo PMA_getLanguageSelectorHtml();
+ echo PMA_Util::getImage('s_lang.png') . " " . PMA_getLanguageSelectorHtml();
echo '</li>';
}
// ThemeManager if available
if ($GLOBALS['cfg']['ThemeManager']) {
- echo '<li id="li_select_theme">';
- echo $_SESSION['PMA_Theme_Manager']->getHtmlSelectBox();
+ echo '<li id="li_select_theme" class="no_bullets">';
+ echo PMA_Util::getImage('s_theme.png') . " "
+ . $_SESSION['PMA_Theme_Manager']->getHtmlSelectBox();
echo '</li>';
}
echo '<li id="li_select_fontsize">';
@@ -207,9 +209,13 @@
if ($server > 0) {
echo '<ul>';
echo PMA_printListItem(
- __('More settings'),
+ PMA_Util::getImage('b_tblops.png')." " .__('More settings'),
'li_user_preferences',
- 'prefs_manage.php?' . $common_url_query
+ 'prefs_manage.php?' . $common_url_query,
+ null,
+ null,
+ null,
+ "no_bullets"
);
echo '</ul>';
}
View
25 libraries/Message.class.php
@@ -639,6 +639,9 @@ public function getMessage()
}
}
+ if ($this->isDisplayed()) {
+ $message = $this->getMessageWithIcon($message);
+ }
if (count($this->getParams()) > 0) {
$message = PMA_Message::format($message, $this->getParams());
}
@@ -720,5 +723,27 @@ public function isDisplayed($isDisplayed = false)
return $this->isDisplayed;
}
+
+ /**
+ * Returns the message with corresponding image icon
+ *
+ * @param string $message the message(s)
+ *
+ * @return string message with icon
+ */
+ public function getMessageWithIcon($message)
+ {
+ $image = '';
+ if ('error' == $this->getLevel()) {
+ $image = 's_error.png';
+ } elseif ('success' == $this->getLevel()) {
+ $image = 's_success.png';
+ } else {
+ $image = 's_notice.png';
+ }
+ $message = PMA_Message::notice(PMA_Util::getImage($image)) . " " . $message;
+ return $message;
+
+ }
}
?>
View
6 libraries/display_create_database.lib.php
@@ -18,7 +18,11 @@
// The user is allowed to create a db
?>
<form method="post" action="db_create.php" id="create_database_form" class="ajax"><strong>
- <?php echo '<label for="text_create_db">' . __('Create database') . '</label>&nbsp;' . PMA_Util::showMySQLDocu('SQL-Syntax', 'CREATE_DATABASE'); ?></strong><br />
+ <?php echo '<label for="text_create_db">'
+ . PMA_Util::getImage('b_newdb.png')
+ . " " . __('Create database')
+ . '</label>&nbsp;'
+ . PMA_Util::showMySQLDocu('SQL-Syntax', 'CREATE_DATABASE'); ?></strong><br />
<?php echo PMA_generate_common_hidden_inputs('', '', 5); ?>
<input type="hidden" name="reload" value="1" />
<input type="text" name="new_db" value="<?php echo $db_to_create; ?>" maxlength="64" class="textfield" id="text_create_db"/>
View
2  server_databases.php
@@ -142,7 +142,7 @@
* Create database.
*/
if ($cfg['ShowCreateDb']) {
- echo '<ul><li id="li_create_database">' . "\n";
+ echo '<ul><li id="li_create_database" class="no_bullets">' . "\n";
include 'libraries/display_create_database.lib.php';
echo ' </li>' . "\n";
echo '</ul>' . "\n";
View
22 test/classes/PMA_Message_test.php
@@ -504,7 +504,7 @@ public function testDisplay()
$this->assertFalse($this->object->isDisplayed());
$this->object->setMessage('Test Message');
- $this->expectOutputString('<div class="notice">Test Message</div>');
+ $this->expectOutputString('<div class="notice"><img src="theme/s_notice.png" title="" alt="" /> Test Message</div>');
$this->object->display();
$this->assertTrue($this->object->isDisplayed());
@@ -519,7 +519,7 @@ public function testGetDisplay()
{
$this->object->setMessage('Test Message');
$this->assertEquals(
- '<div class="notice">Test Message</div>',
+ '<div class="notice"><img src="theme/s_notice.png" title="" alt="" /> Test Message</div>',
$this->object->getDisplay()
);
}
@@ -539,9 +539,9 @@ public function testIsDisplayed()
public function providerAffectedRows()
{
return array(
- array(1, '<div class="notice"> 1 row affected.</div>'),
- array(2, '<div class="notice"> 2 rows affected.</div>'),
- array(10000, '<div class="notice"> 10000 rows affected.</div>'),
+ array(1, '<div class="notice"><img src="theme/s_notice.png" title="" alt="" /> 1 row affected.</div>'),
+ array(2, '<div class="notice"><img src="theme/s_notice.png" title="" alt="" /> 2 rows affected.</div>'),
+ array(10000, '<div class="notice"><img src="theme/s_notice.png" title="" alt="" /> 10000 rows affected.</div>'),
);
}
@@ -567,9 +567,9 @@ public function testAffectedRows($rows, $output)
public function providerInsertedRows()
{
return array(
- array(1, '<div class="notice"> 1 row inserted.</div>'),
- array(2, '<div class="notice"> 2 rows inserted.</div>'),
- array(100000, '<div class="notice"> 100000 rows inserted.</div>'),
+ array(1, '<div class="notice"><img src="theme/s_notice.png" title="" alt="" /> 1 row inserted.</div>'),
+ array(2, '<div class="notice"><img src="theme/s_notice.png" title="" alt="" /> 2 rows inserted.</div>'),
+ array(100000, '<div class="notice"><img src="theme/s_notice.png" title="" alt="" /> 100000 rows inserted.</div>'),
);
}
@@ -595,9 +595,9 @@ public function testInsertedRows($rows, $output)
public function providerDeletedRows()
{
return array(
- array(1, '<div class="notice"> 1 row deleted.</div>'),
- array(2, '<div class="notice"> 2 rows deleted.</div>'),
- array(500000, '<div class="notice"> 500000 rows deleted.</div>'),
+ array(1, '<div class="notice"><img src="theme/s_notice.png" title="" alt="" /> 1 row deleted.</div>'),
+ array(2, '<div class="notice"><img src="theme/s_notice.png" title="" alt="" /> 2 rows deleted.</div>'),
+ array(500000, '<div class="notice"><img src="theme/s_notice.png" title="" alt="" /> 500000 rows deleted.</div>'),
);
}
View
112 themes/original/css/common.css.php
@@ -487,15 +487,6 @@
h1.success,
div.success {
border-color: #00FF00;
- background-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_success.png');?>);
- background-repeat: no-repeat;
- <?php if ($GLOBALS['text_dir'] === 'ltr') { ?>
- background-position: 5px 50%;
- padding: 0.2em 0.2em 0.2em 25px;
- <?php } else { ?>
- background-position: 99% 50%;
- padding: 0.2em 35px 0.2em 0.2em;
- <?php } ?>
}
.success h1 {
border-color: #00FF00;
@@ -508,15 +499,6 @@
h1.notice,
div.notice {
border-color: #FFD700;
- background-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_notice.png');?>);
- background-repeat: no-repeat;
- <?php if ($GLOBALS['text_dir'] === 'ltr') { ?>
- background-position: 5px 50%;
- padding: 0.2em 0.2em 0.2em 25px;
- <?php } else { ?>
- background-position: 99% 50%;
- padding: 0.2em 35px 0.2em 0.2em;
- <?php } ?>
}
.notice h1 {
border-color: #FFD700;
@@ -530,15 +512,6 @@
h1.error,
div.error {
border-color: #ff0000;
- background-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_error.png');?>);
- background-repeat: no-repeat;
- <?php if ($GLOBALS['text_dir'] === 'ltr') { ?>
- background-position: 5px 50%;
- padding: 0.2em 0.2em 0.2em 25px;
- <?php } else { ?>
- background-position: 99% 50%;
- padding: 0.2em 35px 0.2em 0.2em;
- <?php } ?>
}
div.error h1 {
border-color: #ff0000;
@@ -1279,90 +1252,15 @@
}
/* END main page */
-
/* iconic view for ul items */
-li#li_create_database {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_newdb.png');?>);
-}
-
-li#li_select_lang {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_lang.png');?>);
-}
-
-li#li_select_mysql_collation {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_asci.png');?>);
-}
-
-li#li_select_theme {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_theme.png');?>);
-}
-
-li#li_user_info {
- /* list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_rights.png');?>); */
-}
-li#li_mysql_status {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_status.png');?>);
+li.no_bullets {
+ list-style-type:none !important;
+ margin-left: -25px !important; //align with other list items which have bullets
}
-li#li_mysql_variables {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_vars.png');?>);
-}
-
-li#li_mysql_processes {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_process.png');?>);
-}
-
-li#li_mysql_collations {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_asci.png');?>);
-}
-
-li#li_mysql_engines {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_engine.png');?>);
-}
-
-li#li_mysql_binlogs {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_tbl.png');?>);
-}
-
-li#li_mysql_databases {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_db.png');?>);
-}
-
-li#li_export {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_export.png');?>);
-}
-
-li#li_import {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_import.png');?>);
-}
-
-li#li_change_password {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_passwd.png');?>);
-}
-
-li#li_log_out {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_loggoff.png');?>);
-}
-
-li#li_mysql_privilegs {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_rights.png');?>);
-}
-
-li#li_switch_dbstats {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_dbstatistics.png');?>);
-}
-
-li#li_flush_privileges {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_reload.png');?>);
-}
-
-li#li_user_preferences {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_tblops.png');?>);
-}
/* END iconic view for ul items */
-
#body_browse_foreigners {
background: <?php echo $GLOBALS['cfg']['NaviBackground']; ?>;
margin: .5em .5em 0 .5em;
@@ -1490,10 +1388,6 @@
display: none;
}
-#li_select_server {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_host.png');?>);
-}
-
#list_server {
list-style-image: none;
}
View
111 themes/pmahomme/css/common.css.php
@@ -701,7 +701,7 @@
background-repeat: no-repeat;
<?php if ($GLOBALS['text_dir'] === 'ltr') { ?>
background-position: 10px 50%;
- padding: 10px 10px 10px 25px;
+ padding: 10px 10px 10px 10px;
<?php } else { ?>
background-position: 99% 50%;
padding: 10px 35px 10px 10px;
@@ -729,14 +729,7 @@
h1.success,
div.success {
- border-color: #a2d246;
- background-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_success.png');?>);
- background-repeat: no-repeat;
- <?php if ($GLOBALS['text_dir'] === 'ltr') { ?>
- background-position: 5px 50%;
- <?php } else { ?>
- background-position: 99% 50%;
- <?php } ?>
+ border-color: #a2d246;
}
.success h1 {
border-color: #00FF00;
@@ -750,13 +743,6 @@
h1.notice,
div.notice {
border-color: #3a6c7e;
- background-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_notice.png');?>);
- background-repeat: no-repeat;
- <?php if ($GLOBALS['text_dir'] === 'ltr') { ?>
- background-position: 5px 50%;
- <?php } else { ?>
- background-position: 99% 50%;
- <?php } ?>
}
.notice h1 {
@@ -771,14 +757,7 @@
h1.error,
div.error {
- border-color: #333;
- background-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_error.png');?>);
- background-repeat: no-repeat;
- <?php if ($GLOBALS['text_dir'] === 'ltr') { ?>
- background-position: 5px 50%;
- <?php } else { ?>
- background-position: 99% 50%;
- <?php } ?>
+ border-color: #333;
}
div.error h1 {
@@ -1651,88 +1630,14 @@
/* iconic view for ul items */
-li#li_create_database {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_newdb.png');?>);
-}
-
-li#li_select_lang {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_lang.png');?>);
-}
-
-li#li_select_mysql_collation {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_asci.png');?>);
-}
-li#li_select_theme {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_theme.png');?>);
+li.no_bullets {
+ list-style-type:none !important;
+ margin-left: -25px !important; //align with other list items which have bullets
}
-li#li_user_info {
- /* list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_rights.png');?>); */
-}
-
-li#li_mysql_status {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_status.png');?>);
-}
-
-li#li_mysql_variables {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_vars.png');?>);
-}
-
-li#li_mysql_processes {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_process.png');?>);
-}
-
-li#li_mysql_collations {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_asci.png');?>);
-}
-
-li#li_mysql_engines {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_engine.png');?>);
-}
-
-li#li_mysql_binlogs {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_tbl.png');?>);
-}
-
-li#li_mysql_databases {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_db.png');?>);
-}
-
-li#li_export {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_export.png');?>);
-}
-
-li#li_import {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_import.png');?>);
-}
-
-li#li_change_password {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_passwd.png');?>);
-}
-
-li#li_log_out {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_loggoff.png');?>);
-}
-
-li#li_mysql_privilegs {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_rights.png');?>);
-}
-
-li#li_switch_dbstats {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_dbstatistics.png');?>);
-}
-
-li#li_flush_privileges {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_reload.png');?>);
-}
-
-li#li_user_preferences {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('b_tblops.png');?>);
-}
/* END iconic view for ul items */
-
#body_browse_foreigners {
background: <?php echo $GLOBALS['cfg']['NaviBackground']; ?>;
margin: .5em .5em 0 .5em;
@@ -1874,10 +1779,6 @@
display: none;
}
-#li_select_server {
- list-style-image: url(<?php echo $_SESSION['PMA_Theme']->getImgPath('s_host.png');?>);
-}
-
#list_server {
list-style-image: none;
}
Something went wrong with that request. Please try again.