Skip to content

Commit

Permalink
do not allow persons modal opening on dashboard item view (#5286)
Browse files Browse the repository at this point in the history
  • Loading branch information
liyiy authored Jul 22, 2021
1 parent d7e4563 commit bfb1a00
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 11 deletions.
16 changes: 10 additions & 6 deletions frontend/src/scenes/funnels/FunnelBarGraph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ interface BarProps {
percentage: number
name?: string
onBarClick?: () => void
disabled?: boolean
layout?: FunnelLayout
isBreakdown?: boolean
breakdownIndex?: number
Expand All @@ -43,6 +44,7 @@ function Bar({
percentage,
name,
onBarClick,
disabled,
layout = FunnelLayout.horizontal,
isBreakdown = false,
breakdownIndex,
Expand All @@ -56,9 +58,9 @@ function Bar({
const [labelPosition, setLabelPosition] = useState<LabelPosition>('inside')
const [labelVisible, setLabelVisible] = useState(true)
const LABEL_POSITION_OFFSET = 8 // Defined here and in SCSS
const { funnelPersonsEnabled } = useValues(funnelLogic)
const { clickhouseFeaturesEnabled } = useValues(funnelLogic)
const dimensionProperty = layout === FunnelLayout.horizontal ? 'width' : 'height'
const cursorType = funnelPersonsEnabled ? 'pointer' : ''
const cursorType = clickhouseFeaturesEnabled && !disabled ? 'pointer' : ''
const hasBreakdownSum = isBreakdown && typeof breakdownSumPercentage === 'number'
const shouldShowLabel = !isBreakdown || (hasBreakdownSum && labelVisible)

Expand Down Expand Up @@ -140,7 +142,7 @@ function Bar({
backgroundColor: getSeriesColor(breakdownIndex),
}}
onClick={() => {
if (funnelPersonsEnabled && onBarClick) {
if (clickhouseFeaturesEnabled && !disabled && onBarClick) {
onBarClick()
}
}}
Expand Down Expand Up @@ -275,7 +277,7 @@ function MetricRow({ title, value }: { title: string; value: string | number }):

export function FunnelBarGraph({ filters, dashboardItemId, color = 'white' }: Omit<ChartParams, 'view'>): JSX.Element {
const logic = funnelLogic({ dashboardItemId, filters })
const { steps, stepReference, barGraphLayout: layout, funnelPersonsEnabled } = useValues(logic)
const { steps, stepReference, barGraphLayout: layout, clickhouseFeaturesEnabled } = useValues(logic)
const { openPersonsModal } = useActions(funnelLogic)
const firstStep = getReferenceStep(steps, FunnelStepReference.total)

Expand Down Expand Up @@ -348,6 +350,7 @@ export function FunnelBarGraph({ filters, dashboardItemId, color = 'white' }: Om
percentage={conversionRate}
name={breakdown.name}
onBarClick={() => openPersonsModal(step, i + 1, step.breakdown_value)}
disabled={!!dashboardItemId}
layout={layout}
popoverTitle={
<div style={{ wordWrap: 'break-word' }}>
Expand Down Expand Up @@ -398,6 +401,7 @@ export function FunnelBarGraph({ filters, dashboardItemId, color = 'white' }: Om
percentage={calcPercentage(step.count, basisStep.count)}
name={step.name}
onBarClick={() => openPersonsModal(step, i + 1)}
disabled={!!dashboardItemId}
layout={layout}
popoverTitle={<PropertyKeyInfo value={step.name} />}
popoverMetrics={[
Expand Down Expand Up @@ -447,7 +451,7 @@ export function FunnelBarGraph({ filters, dashboardItemId, color = 'white' }: Om
<div className="center-flex">
<ValueInspectorButton
onClick={() => openPersonsModal(step, i + 1)}
disabled={!funnelPersonsEnabled}
disabled={!clickhouseFeaturesEnabled || !!dashboardItemId}
>
<span className="value-inspector-button-icon">
<ArrowRightOutlined style={{ color: 'var(--success)' }} />
Expand All @@ -468,7 +472,7 @@ export function FunnelBarGraph({ filters, dashboardItemId, color = 'white' }: Om
<div className="center-flex">
<ValueInspectorButton
onClick={() => openPersonsModal(step, -(i + 1))} // dropoff value from step 1 to 2 is -2, 2 to 3 is -3
disabled={!funnelPersonsEnabled}
disabled={!clickhouseFeaturesEnabled || !!dashboardItemId}
style={{ paddingRight: '0.25em' }}
>
<span
Expand Down
5 changes: 0 additions & 5 deletions frontend/src/scenes/funnels/funnelLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,11 +318,6 @@ export const funnelLogic = kea<funnelLogicType>({
(featureFlags, preflight): boolean =>
!!(featureFlags[FEATURE_FLAGS.FUNNEL_BAR_VIZ] && preflight?.is_clickhouse_enabled),
],
funnelPersonsEnabled: [
() => [featureFlagLogic.selectors.featureFlags, selectors.preflight],
(featureFlags, preflight): boolean =>
!!(featureFlags[FEATURE_FLAGS.FUNNEL_BAR_VIZ] && preflight?.is_clickhouse_enabled),
],
histogramGraphData: [
() => [selectors.timeConversionBins],
(timeConversionBins: FunnelsTimeConversionBins) => {
Expand Down

0 comments on commit bfb1a00

Please sign in to comment.