-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add reorder to dashboard parameter widgets #5267
Merged
Merged
Changes from all commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
65b9e17
added paramOrder prop
rafawendel 41c7ff4
minor refactor
rafawendel 5a53e62
Merge branch 'master' into reorder-dashboard-parameters
rafawendel 8b8ad55
moved logic to widget
rafawendel 6fbbacf
Added paramOrder to widget API call
rafawendel 21e4b53
Update client/app/components/dashboards/dashboard-widget/Visualizatio…
rafawendel 7c5f135
Merge branch 'master' into reorder-dashboard-parameters
rafawendel c82b8bb
Merge branch 'master' into reorder-dashboard-parameters
rafawendel 233dff9
experimental removal of helper element
rafawendel 0b7e123
cleaner comment
rafawendel dec8cfe
Added dashboard global params logic
rafawendel 41ae2ce
Added backend logic for dashboard options
rafawendel 9e01b2d
Removed testing leftovers
rafawendel 163fb6f
removed appending sortable to parent component behavior
rafawendel 026608a
Revert "Added backend logic for dashboard options"
rafawendel 19acbeb
Merge branch 'master' of github.com:getredash/redash into reorder-das…
rafawendel 036b9ed
Re-structured backend options
rafawendel 6071bef
removed temporary edits
rafawendel 232d828
Added dashboard/widget param reorder cypress tests
rafawendel ee94809
Separated edit and sorting permission
rafawendel ada5ccf
added options to public dashboard serializer
rafawendel 16275c1
Merge branch 'master' of github.com:getredash/redash into reorder-das…
rafawendel 019883a
Removed undesirable events from drag
rafawendel 975ddec
Bring back attaching sortable to its parent
rafawendel 4952605
Added prop to control draggable destination parent
rafawendel a0c1c0e
Removed paramOrder fallback
rafawendel 433e11e
WIP (for Netflify preview)
rafawendel 5986743
fixup! Added prop to control draggable destination parent
rafawendel 027ee86
Better drag and drop styling and fix for the padding
rafawendel 8689e0e
Revert "WIP (for Netflify preview)"
rafawendel 4a4cc0a
Improved dashboard parameter Cypress test
rafawendel 03d806a
Standardized reorder styling
rafawendel 2c68d18
Changed dashboard param reorder to edit mode only
rafawendel af0d2af
fixup! Improved dashboard parameter Cypress test
rafawendel 83d3eab
fixup! Improved dashboard parameter Cypress test
rafawendel e53aa8d
Fix for Cypress CI error
rafawendel 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import { isEmpty } from "lodash"; | ||
import { isEmpty, map } from "lodash"; | ||
import React, { useState, useEffect } from "react"; | ||
import PropTypes from "prop-types"; | ||
import cx from "classnames"; | ||
|
@@ -24,8 +24,8 @@ import DashboardHeader from "./components/DashboardHeader"; | |
|
||
import "./DashboardPage.less"; | ||
|
||
function DashboardSettings({ dashboardOptions }) { | ||
const { dashboard, updateDashboard } = dashboardOptions; | ||
function DashboardSettings({ dashboardConfiguration }) { | ||
const { dashboard, updateDashboard } = dashboardConfiguration; | ||
return ( | ||
<div className="m-b-10 p-15 bg-white tiled"> | ||
<Checkbox | ||
|
@@ -39,11 +39,11 @@ function DashboardSettings({ dashboardOptions }) { | |
} | ||
|
||
DashboardSettings.propTypes = { | ||
dashboardOptions: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types | ||
dashboardConfiguration: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types | ||
}; | ||
|
||
function AddWidgetContainer({ dashboardOptions, className, ...props }) { | ||
const { showAddTextboxDialog, showAddWidgetDialog } = dashboardOptions; | ||
function AddWidgetContainer({ dashboardConfiguration, className, ...props }) { | ||
const { showAddTextboxDialog, showAddWidgetDialog } = dashboardConfiguration; | ||
return ( | ||
<div className={cx("add-widget-container", className)} {...props}> | ||
<h2> | ||
|
@@ -66,12 +66,12 @@ function AddWidgetContainer({ dashboardOptions, className, ...props }) { | |
} | ||
|
||
AddWidgetContainer.propTypes = { | ||
dashboardOptions: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types | ||
dashboardConfiguration: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types | ||
className: PropTypes.string, | ||
}; | ||
|
||
function DashboardComponent(props) { | ||
const dashboardOptions = useDashboard(props.dashboard); | ||
const dashboardConfiguration = useDashboard(props.dashboard); | ||
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. Renamed this to avoid confusion |
||
const { | ||
dashboard, | ||
filters, | ||
|
@@ -81,14 +81,19 @@ function DashboardComponent(props) { | |
removeWidget, | ||
saveDashboardLayout, | ||
globalParameters, | ||
updateDashboard, | ||
refreshDashboard, | ||
refreshWidget, | ||
editingLayout, | ||
setGridDisabled, | ||
} = dashboardOptions; | ||
} = dashboardConfiguration; | ||
|
||
const [pageContainer, setPageContainer] = useState(null); | ||
const [bottomPanelStyles, setBottomPanelStyles] = useState({}); | ||
const onParametersEdit = parameters => { | ||
const paramOrder = map(parameters, "name"); | ||
updateDashboard({ options: { globalParamOrder: paramOrder } }); | ||
}; | ||
|
||
useEffect(() => { | ||
if (pageContainer) { | ||
|
@@ -114,22 +119,31 @@ function DashboardComponent(props) { | |
return ( | ||
<div className="container" ref={setPageContainer} data-test={`DashboardId${dashboard.id}Container`}> | ||
<DashboardHeader | ||
dashboardOptions={dashboardOptions} | ||
dashboardConfiguration={dashboardConfiguration} | ||
headerExtra={ | ||
<DynamicComponent name="Dashboard.HeaderExtra" dashboard={dashboard} dashboardOptions={dashboardOptions} /> | ||
<DynamicComponent | ||
name="Dashboard.HeaderExtra" | ||
dashboard={dashboard} | ||
dashboardConfiguration={dashboardConfiguration} | ||
/> | ||
} | ||
/> | ||
{!isEmpty(globalParameters) && ( | ||
<div className="dashboard-parameters m-b-10 p-15 bg-white tiled" data-test="DashboardParameters"> | ||
<Parameters parameters={globalParameters} onValuesChange={refreshDashboard} /> | ||
<Parameters | ||
parameters={globalParameters} | ||
onValuesChange={refreshDashboard} | ||
sortable={editingLayout} | ||
onParametersEdit={onParametersEdit} | ||
/> | ||
</div> | ||
)} | ||
{!isEmpty(filters) && ( | ||
<div className="m-b-10 p-15 bg-white tiled" data-test="DashboardFilters"> | ||
<Filters filters={filters} onChange={setFilters} /> | ||
</div> | ||
)} | ||
{editingLayout && <DashboardSettings dashboardOptions={dashboardOptions} />} | ||
{editingLayout && <DashboardSettings dashboardConfiguration={dashboardConfiguration} />} | ||
<div id="dashboard-container"> | ||
<DashboardGrid | ||
dashboard={dashboard} | ||
|
@@ -144,7 +158,9 @@ function DashboardComponent(props) { | |
onParameterMappingsChange={loadDashboard} | ||
/> | ||
</div> | ||
{editingLayout && <AddWidgetContainer dashboardOptions={dashboardOptions} style={bottomPanelStyles} />} | ||
{editingLayout && ( | ||
<AddWidgetContainer dashboardConfiguration={dashboardConfiguration} style={bottomPanelStyles} /> | ||
)} | ||
</div> | ||
); | ||
} | ||
|
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.
What exactly does this fix?
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.
By standard React Sortable HOC will append the draggable to the document body. The creator of our own component decided to add a container to it, the reason for which wasn't clear to me, until I removed it and noticed minor undesirable UI changes. I brought it back and added this as an alternative to only change what needs to be.
In the case of widgets, it's necessary to append them to a component in an upper scope than that of their containers, as they are positioned using transforms, which mess with the final position of the draggable. I tried to workaround them, but it got too complex and this seemed clearer.