Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADD icons for priority #224

Merged
merged 2 commits into from
Dec 13, 2016
Merged

ADD icons for priority #224

merged 2 commits into from
Dec 13, 2016

Conversation

phavel
Copy link
Contributor

@phavel phavel commented Dec 1, 2016

Proposal icon for priority.

Icons: Farm-fresh http://www.fatcow.com/free-icons
DB: add "pri_icon" column in priority table

list
view
manage-1
manage-2

@glensc
Copy link
Member

glensc commented Dec 3, 2016

schema.sql may not be modified. if you need db chage, add new file (in sequence) to upgrade/patches dir

@@ -89,7 +89,7 @@
{if $field_name == 'iss_id'}
<a href="view.php?id={$list[i].iss_id}" title="{t}view issue details{/t}">{$list[i].iss_id}</a>
{elseif $field_name == 'pri_rank'}
{$list[i].pri_title|escape:"html"}
{if $list[i].pri_icon > 0}<img src="{$core.rel_url}images/priority/{$list[i].pri_icon|escape:"html"}.png" border="0" align="absmiddle"> {/if}{$list[i].pri_title|escape:"html"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather than writing image url in html, add appropriate css class instead

$(function() {
$.widget( "custom.iconselectmenu", $.ui.selectmenu, {
_renderItem: function( ul, item ) {
var li = $( "<li>", { text: item.label } );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use convention that jQuery selector variables start with a dollar ($)

}
.icon .ui-icon {
background-position: left top;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not inline css. add to global css file instead. page.css should have page specific css selectors

@@ -80,6 +80,8 @@
(<a href="#customer_details">{t}Complete Details{/t}</a>)
{elseif $row.field == 'percentage_complete'}
<div class="ui-progressbar iss_percent_complete" data-percent="{$row.percent}"><div class="progress-label">&nbsp;{$row.percent} %</div></div>
{elseif $row.field == 'priority'}
{if $row.pri_icon > 0}<img src="{$core.rel_url}images/priority/{$row.pri_icon|escape:"html"}.png" border="0" align="absmiddle"> {/if}{$row.pri_title|escape:"html"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indenting broken. also try to use css not hardcode urls into html.

also, can pri_icon value be 0 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pri_icon = 0 is default (in DB), it mean none icon.

@glensc
Copy link
Member

glensc commented Dec 8, 2016

what's with your commit messages? please write sane commit messages not those fixup! if you add those for autosquash, then please squash and push -f instead!

fixup! ADD icons for priority is useless commit message. it should rather describe what the commit will do if applied.

@glensc
Copy link
Member

glensc commented Dec 8, 2016

to fix commit message of last commit:

  • git commit --amend
  • write better commit message
  • git push -f

padding-left: 16px;
margin: 0 3px 0 0px;
}
.icon_1 {background-image: url("../images/priority/1.png");}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is poor global class name.

better one: .priority-icon-1

@phavel
Copy link
Contributor Author

phavel commented Dec 8, 2016

Sorry, I am beginner. Thank for info.

@glensc
Copy link
Member

glensc commented Dec 11, 2016

i get such notices in admin panel:

( ! ) Notice: Undefined index: info in var/cache/8ef96dda9622a12994e2241bc715565f66a00d07_0.file.priorities.tpl.html.php on line 285

and the icon is not shown in admin view

image

@glensc
Copy link
Member

glensc commented Dec 11, 2016

ok, icons appeared after clearing browser cache. but the info errors are still there in admin view

@phavel
Copy link
Contributor Author

phavel commented Dec 11, 2016

I find error and fix. Thanks.

@glensc glensc merged commit 419c9b7 into eventum:master Dec 13, 2016
glensc added a commit that referenced this pull request Dec 13, 2016
glensc added a commit that referenced this pull request Dec 13, 2016
balsdorf pushed a commit that referenced this pull request Jan 13, 2017
@glensc glensc mentioned this pull request May 7, 2017
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants