-
Notifications
You must be signed in to change notification settings - Fork 33
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
Adding heatmap and graph_type #134
Merged
+1,610
−546
Merged
Changes from 6 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
1bc5860
adding heatmap and graph_type
jlampar e326cc4
fixed one-container bug
jlampar 2ba0fed
minor Graph refactoring
jlampar 663693a
integrated chartID, renaming mfa/mfas, filtering
jlampar 0f54638
fixing async issue
jlampar 8e2e0ad
changed re-creation into updating the attributes
jlampar 1596234
changing to redux way and reducing the mess
jlampar daa5b3d
Merge branch 'release_2.0' into addHeatmap
jlampar 22ebea0
fixing imports
jlampar f064018
Merge branch 'addHeatmap' of https://github.com/Appliscale/xprof into…
jlampar 2f6b271
fixing imports and line length
jlampar d79246b
still fixing imports
jlampar 76d4281
fixing MC being undefined
jlampar 8eb3ffb
reverting
jlampar 1cc4587
fixing test to fit new return format
jlampar d4247d1
fixing indentation
jlampar 44ee97b
fixing
jlampar 29f53da
fixing roll assignement and labels positioning
jlampar ebe69ad
experiment
jlampar 39c9f49
checking for data
jlampar c188f8c
resolving conflicts
jlampar 322b7c1
adding label tooltip, id assignement
jlampar 8c08420
removing some comments
jlampar 92d561c
changing the cursors
jlampar bf6c4e9
moving safecomposeID to state
jlampar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,21 +3,55 @@ import React from 'react'; | |
import C3Chart from 'react-c3js'; | ||
import 'c3/c3.css'; | ||
import { AXIS, DATA, GRID, POINT, TRANSITION } from '../../../constants'; | ||
import Grid from './Grid/Grid'; | ||
|
||
const composeID = q => `${q.replace(/[^A-Za-z0-9_-]/g, '-')}`; | ||
|
||
const propTypes = { | ||
dps: PropTypes.arrayOf(PropTypes.object).isRequired, | ||
type: PropTypes.string.isRequired, | ||
query: PropTypes.string.isRequired, | ||
passId: PropTypes.number.isRequired, | ||
}; | ||
|
||
const Graph = ({ | ||
dps, | ||
type, | ||
query, | ||
passId, | ||
}) => { | ||
const queryID = composeID(query); | ||
const wrapperID = `graphWrapper-${queryID}`; | ||
const wrapper = document.getElementById(wrapperID); | ||
if (type === 'grid') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can create constant for types and put it somewhere in
|
||
return ( | ||
<div | ||
id={wrapperID} | ||
style={{ | ||
textAlign: 'center', | ||
justifyContent: 'center', | ||
}} | ||
> | ||
{wrapper && <Grid | ||
data={{ ...DATA, json: dps }} | ||
outerWidth={wrapper.clientWidth} | ||
graphID={passId} | ||
/>} | ||
</div> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we introduced new type of graph we can add new layer of components here
|
||
); | ||
} | ||
return ( | ||
<div> | ||
<C3Chart | ||
data={{ ...DATA, json: dps }} | ||
point={POINT} | ||
grid={GRID} | ||
axis={AXIS} | ||
transition={TRANSITION} | ||
/> | ||
</div> | ||
); | ||
}; | ||
const Graph = ({ dps }) => ( | ||
<div> | ||
<C3Chart | ||
data={{ ...DATA, json: dps }} | ||
point={POINT} | ||
grid={GRID} | ||
axis={AXIS} | ||
transition={TRANSITION} | ||
/> | ||
</div> | ||
); | ||
Graph.propTypes = propTypes; | ||
|
||
export default Graph; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this can be the whole
m
json object (includingmfa
andquery
fields) to address above issue with composing the ID(we need to find a good name for the json object, I dont know if
function
is good enough, in generic sense this is amonitor
ormonitored-object
, which can be an m:f/a but also can be garbage_collection or memory in the future)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are composing the ID inside
GraphPanel
, in the end we are only passing the ID to theGraph
and the panel has access to the wholem
object. The panel needs the object as it decomposes it into the grid props, whereas the grid just receives stuff from the panel.In case of renaming the JSON object I think this is a good idea, as for now the object named
mfa
has also amfa
property which is a little bit confusing (that's why I renamed it somewhere tom
in loops or arguments).I propose we:
mfas
tomonitoredCollection
,mfa
tomonitored
.What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, so
query
is only there so that theGraph
can show it as a title. +1 for renamings