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

Change text color to blue for different columns when displaying Drift for VM #3938

Merged
merged 1 commit into from May 16, 2018

Conversation

hstastna
Copy link
Contributor

Before:
blue_before

After:
blue_after

@miq-bot miq-bot added the wip label May 14, 2018
@hstastna hstastna changed the title [WIP] Change text color to blue for different columns when displaying Drift for VM Change text color to blue for different columns when displaying Drift for VM May 14, 2018
@hstastna
Copy link
Contributor Author

@dclarizio This PR is related to one of the issues you were talking about, when talking about Drifts and VMs with me ;) What do you think about it?

@miq-bot miq-bot removed the wip label May 14, 2018
@hstastna hstastna force-pushed the Drift_diff_color_blue branch 2 times, most recently from 039e513 to 9b54973 Compare May 14, 2018 22:19
def drift_add_txt_col(idx, col, img_bkg)
html_text = "<div class='#{img_bkg}'>#{col}</div>"
def drift_add_txt_col(idx, col, img_bkg, style = nil)
html_text = if style.present?
Copy link
Member

Choose a reason for hiding this comment

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

... if style

if idx.zero? # On base object
img_bkg = "cell-stripe"
elsif fld[:_match_]
img_bkg = "cell-bkg-plain-no-shade"
else
style = "color: #21a0ec;" if fld[:_match_] == false
Copy link
Member

Choose a reason for hiding this comment

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

... unless fld[:_match__]

Copy link
Contributor Author

@hstastna hstastna May 15, 2018

Choose a reason for hiding this comment

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

Here we want to set style ONLY when there IS _match_ key and if it is set to false. Often, fld looks like this: {:_value_=>"(empty)"}, and then if I used unless fld[:_match_], style would be set, and we don't want this in this case. So I better used condition in format if fld[:_match_] == false, to make sure that the style is set really only when we want it. Fortunatelly, when fld has no _match_ key, idx is usually zero so it will not enter the else part of the block and nothing bad should happen, in fact. But I don't want to rely on this (on the fact that idx is checked before checking fld).

Copy link
Member

Choose a reason for hiding this comment

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

okay

@@ -1208,15 +1208,17 @@ def drift_add_section_field_expanded(view, section, field)
next if fld.nil?
val = fld[:_value_].to_s

style = nil
Copy link
Member

Choose a reason for hiding this comment

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

YAGNI

@miq-bot
Copy link
Member

miq-bot commented May 15, 2018

Checked commit hstastna@e86a108 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

The Seal of Approval

@mzazrivec mzazrivec self-assigned this May 16, 2018
@mzazrivec mzazrivec added this to the Sprint 86 Ending May 21, 2018 milestone May 16, 2018
@mzazrivec mzazrivec merged commit 61143ca into ManageIQ:master May 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants