-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: Image download #1931
feat: Image download #1931
Changes from all commits
615c1dc
85e46a4
2ef7911
90c905f
ebdd3c6
53a9efa
5864301
dd3c3d0
7668511
98b957c
1723945
3506fac
f484bf7
9810604
1aacf85
246bf0c
59e2be3
af03396
d6ad38a
7571dc9
cd3eb7c
e65ec30
d38201c
506de01
75fd1fb
b1cc7fc
d6e9c49
1a8aa2c
0c4c21c
ae9492d
e9dba7b
d66cec2
c8df4a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -287,6 +287,20 @@ export const extractChartConfigComponentIds = ({ | |
); | ||
}; | ||
|
||
export const extractChartConfigUsedComponents = ( | ||
chartConfig: ChartConfig, | ||
{ components }: { components: Component[] } | ||
) => { | ||
const componentIds = extractChartConfigComponentIds({ | ||
chartConfig, | ||
includeFilters: false, | ||
}); | ||
|
||
return componentIds | ||
.map((id) => components.find((component) => component.id === id)) | ||
.filter(truthy); // exclude potential joinBy components | ||
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. Do we still need this comment? 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. Here I would also say yes, as I wouldn't know why we need to use const excludeJoinByComponent = (c: Component) => truthy(c); but I think a comment should suffice here 👍 It was a bit of a pain to catch all the edge-cases of merging of cubes, I'd rather keep more context in places related to them 🥹 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. I think comments that describe a "clear" logic should be generally removed, like this: // Find a component
const component = components.find(c => c.id === id); but when they are related to other, connected parts of the code or generally are not "obvious", it's okay to use them 👍 |
||
}; | ||
|
||
/** Use to remove missing values from chart data. */ | ||
export const usePlottableData = ( | ||
data: Observation[], | ||
|
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.
Do we still need this comment?
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.
I would say yes, as
preserveDrawingBuffer
isfalse
by default and can decrease the performance of the map, the comment should make sure that we don't remove it and unknowingly break screenshotting of the maps :)