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

Use proper name of column in tooltip in charts #13011

Merged

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Dec 6, 2016

there was first one taken before it should be taken from Data Column:
screen shot 2016-12-06 at 11 41 36

before
screen shot 2016-12-06 at 11 45 14

after
screen shot 2016-12-06 at 11 41 23

@miq-bot add_label bug, ui, euwe/yes

@miq-bot assign @mzazrivec

please review @PanSpagetka

@lpichler lpichler force-pushed the use_proper_name_for_tooltip_in_charts branch 2 times, most recently from cdae74e to 92260ef Compare December 6, 2016 12:27
@lpichler lpichler changed the title Use proper name of column in tooltip in charts [WIP] Use proper name of column in tooltip in charts Dec 6, 2016
@lpichler
Copy link
Contributor Author

lpichler commented Dec 6, 2016

@miq-bot add_label wip

@miq-bot miq-bot added the wip label Dec 6, 2016
@lpichler lpichler force-pushed the use_proper_name_for_tooltip_in_charts branch 2 times, most recently from 99a720c to e3f3fad Compare December 7, 2016 15:49
@lpichler lpichler changed the title [WIP] Use proper name of column in tooltip in charts Use proper name of column in tooltip in charts Dec 7, 2016
@miq-bot miq-bot removed the wip label Dec 7, 2016
@lpichler
Copy link
Contributor Author

lpichler commented Dec 7, 2016

@PanSpagetka please review

@lpichler lpichler force-pushed the use_proper_name_for_tooltip_in_charts branch from 6255390 to 379d5c4 Compare December 8, 2016 12:02
# determine name column from headers for x-axis in chart
def chart_header_column
if graph[:column].blank?
_log.error("Column for x-axis of chart have to be defined in report")

Choose a reason for hiding this comment

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

Message text should be The column for the chart's x-axis must be defined in the report.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 thanks @dclarizio

- determine name of column (Header column) from chart column.
Name is taken from MiqReport#headers by index.
This index is determined from MiqReport#col_order
@lpichler lpichler force-pushed the use_proper_name_for_tooltip_in_charts branch from 379d5c4 to f002856 Compare December 9, 2016 10:01
@miq-bot
Copy link
Member

miq-bot commented Dec 9, 2016

Checked commits lpichler/manageiq@99cdbaa~...f002856 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
4 files checked, 0 offenses detected
Everything looks good. ⭐

@mzazrivec mzazrivec added this to the Sprint 51 Ending Jan 2, 2017 milestone Dec 13, 2016
@mzazrivec mzazrivec merged commit d6b146f into ManageIQ:master Dec 13, 2016
@simaishi
Copy link
Contributor

simaishi commented Jan 9, 2017

@lpichler is there a BZ for this?

@lpichler lpichler deleted the use_proper_name_for_tooltip_in_charts branch January 11, 2017 16:58
@lpichler
Copy link
Contributor Author

@simaishi No. I found it by myself.

@simaishi
Copy link
Contributor

simaishi pushed a commit that referenced this pull request Jan 12, 2017
…n_charts

Use proper name of column in tooltip in charts
(cherry picked from commit d6b146f)

https://bugzilla.redhat.com/show_bug.cgi?id=1412738
@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit 774d3a2e29ccb63fec4b397e51c9d49e1ab8b460
Author: Milan Zázrivec <[email protected]>
Date:   Tue Dec 13 14:40:47 2016 +0100

    Merge pull request #13011 from lpichler/use_proper_name_for_tooltip_in_charts
    
    Use proper name of column in tooltip in charts
    (cherry picked from commit d6b146f69196b83940b92c527a011167d5f90826)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1412738

@simaishi simaishi removed the euwe/yes label Jan 12, 2017
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.

5 participants