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.

Sign up for GitHub

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

Jump to bottom

[Feature]: Heatmap & AreaGraph Legends #436

Merged
merged 7 commits into from
Jan 12, 2021
Merged

[Feature]: Heatmap & AreaGraph Legends #436

merged 7 commits into from
Jan 12, 2021

Conversation

nxanil
Copy link
Collaborator

@nxanil nxanil commented Jan 7, 2021 •
edited
Loading

@killanch @chaitanyahalbe

Fixed issue #434. Please review this PR.

heatmap

@myTechPlayArea
Copy link
Contributor

@nxanil Is issue 1) which is displaying all available heatmapColor object fixed? I dont see that in the gifs added. Can you please clarify.

@nxanil
Copy link
Collaborator Author

nxanil commented Jan 8, 2021

@nxanil Is issue 1) which is displaying all available heatmapColor object fixed? I don't see that in the gifs added. Can you please clarify.

@myTechPlayArea
That issue is also fixed in this PR. Please check the below mention screenshot.

"heatmapColor": { "0.0% - 0.49%": "#75B35E", "0.5% - 0.99%": "#A8B343", "1.0% - 1.49%": "#FAC956", "1.5% - 1.99%": "#FFA939", "2.0% - 3.99%": "#FF8550", "4.0% - 5.99%": "#FF4949", "6.0% - 9.99%": "#FF0D0D", ">= 10.0%": "#F2F3F4" }

heat

@chaitanyahalbe
Copy link
Contributor

@nxanil Is issue 1) which is displaying all available heatmapColor object fixed? I don't see that in the gifs added. Can you please clarify.

@myTechPlayArea
That issue is also fixed in this PR. Please check the below mention screenshot.

"heatmapColor": { "0.0% - 0.49%": "#75B35E", "0.5% - 0.99%": "#A8B343", "1.0% - 1.49%": "#FAC956", "1.5% - 1.99%": "#FFA939", "2.0% - 3.99%": "#FF8550", "4.0% - 5.99%": "#FF4949", "6.0% - 9.99%": "#FF0D0D", ">= 10.0%": "#F2F3F4" }

heat

@nxanil The Area Graph legend looks fine. But the legend for heatmap should appear in the same order as mentioned in the config file to avoid confusion.

@nxanil
Copy link
Collaborator Author

nxanil commented Jan 11, 2021

@nxanil Is issue 1) which is displaying all available heatmapColor object fixed? I don't see that in the gifs added. Can you please clarify.

@myTechPlayArea
That issue is also fixed in this PR. Please check the below mention screenshot.
"heatmapColor": { "0.0% - 0.49%": "#75B35E", "0.5% - 0.99%": "#A8B343", "1.0% - 1.49%": "#FAC956", "1.5% - 1.99%": "#FFA939", "2.0% - 3.99%": "#FF8550", "4.0% - 5.99%": "#FF4949", "6.0% - 9.99%": "#FF0D0D", ">= 10.0%": "#F2F3F4" }
heat

@nxanil The Area Graph legend looks fine. But the legend for heatmap should appear in the same order as mentioned in the config file to avoid confusion.

@chaitanyahalbe

Changes have been done. Please have a look.

heatmap

@chaitanyahalbe
Copy link
Contributor

@nxanil Is issue 1) which is displaying all available heatmapColor object fixed? I don't see that in the gifs added. Can you please clarify.

@myTechPlayArea
That issue is also fixed in this PR. Please check the below mention screenshot.
"heatmapColor": { "0.0% - 0.49%": "#75B35E", "0.5% - 0.99%": "#A8B343", "1.0% - 1.49%": "#FAC956", "1.5% - 1.99%": "#FFA939", "2.0% - 3.99%": "#FF8550", "4.0% - 5.99%": "#FF4949", "6.0% - 9.99%": "#FF0D0D", ">= 10.0%": "#F2F3F4" }
heat

@nxanil The Area Graph legend looks fine. But the legend for heatmap should appear in the same order as mentioned in the config file to avoid confusion.

@chaitanyahalbe

Changes have been done. Please have a look.

heatmap

@nxanil Looks good.

chaitanyahalbe
chaitanyahalbe approved these changes Jan 11, 2021
View reviewed changes
Copy link
Contributor

@chaitanyahalbe chaitanyahalbe left a comment

Choose a reason for hiding this comment

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

Looks good.

killanch
killanch reviewed Jan 11, 2021
View reviewed changes
Graphs/AbstractGraph.js Outdated
@@ -547,19 +547,29 @@ export default class AbstractGraph extends React.Component {
return legend.circleSize * circleToPixel;
}

renderLegend(data, legend, getColor, label, isVertical) {
renderLegend(data, legend, getColor, label, isVertical, heatmapColor) {
Copy link
Contributor

Choose a reason for hiding this comment

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

heatmapColor = {}
pls fix other such code as well, null heatmapColor should not result in exception

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@killanch

Changes have been made. Please check once.

@killanch
Copy link
Contributor

@chaitanyahalbe can you pls merge and bring in to vsd-react-ui 0.0 and 20.10?

@chaitanyahalbe chaitanyahalbe merged commit e6a631a into master Jan 12, 2021
@chaitanyahalbe chaitanyahalbe deleted the enhancement/change-legends branch January 12, 2021 23:25
@chaitanyahalbe chaitanyahalbe mentioned this pull request Jan 12, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@killanch killanch killanch approved these changes

@chaitanyahalbe chaitanyahalbe chaitanyahalbe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nxanil @myTechPlayArea @chaitanyahalbe @killanch