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.

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

Remove query and site props passing from everywhere except class components #4340

Merged
merged 2 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions assets/js/dashboard/comparison-input.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ function ComparisonModeOption({ label, value, isCurrentlySelected, updateMode, s
)
}

function MatchDayOfWeekInput({ history, query, site }) {
function MatchDayOfWeekInput({ history }) {
const site = useSiteContext()
const { query } = useQueryContext()
Copy link
Contributor

Choose a reason for hiding this comment

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

Would something like useDashboardContext() make sense which would encapsulate both site AND query? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

I considered it initially, but my hunch is that many of these components don't really need either.

I think the way forward is to actually remove some of these superfluous dependencies, and provide components with specific handlers for what they're supposed to do: in this example handleCompareExactMatch, handleCompareDayOfTheWeek.

Such handlers would be declared higher up in the React tree and themselves dependent on a shared API component, which is dependent on the site.domain.

const click = (matchDayOfWeek) => {
storage.setItem(`comparison_match_day_of_week__${site.domain}`, matchDayOfWeek.toString())
navigateToQuery(history, query, { match_day_of_week: matchDayOfWeek.toString() })
Expand Down Expand Up @@ -184,7 +186,7 @@ const ComparisonInput = function ({ history }) {
{Object.keys(COMPARISON_MODES).map((key) => ComparisonModeOption({ label: COMPARISON_MODES[key], value: key, isCurrentlySelected: key == query.comparison, updateMode, setUiMode }))}
{query.comparison !== "custom" && <span>
<hr className="my-1" />
<MatchDayOfWeekInput query={query} history={history} site={site} />
<MatchDayOfWeekInput history={history} />
</span>}
</Menu.Items>
</Transition>
Expand Down
4 changes: 3 additions & 1 deletion assets/js/dashboard/components/notice.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import React from "react"
import { sectionTitles } from "../stats/behaviours"
import * as api from '../api'
import { useSiteContext } from "../site-context"

export function FeatureSetupNotice({ site, feature, title, info, callToAction, onHideAction }) {
export function FeatureSetupNotice({ feature, title, info, callToAction, onHideAction }) {
const site = useSiteContext()
const sectionTitle = sectionTitles[feature]

const requestHideSection = () => {
Expand Down
25 changes: 13 additions & 12 deletions assets/js/dashboard/datepicker.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import React, { Fragment, useState, useEffect, useCallback, useRef } from "react";
import { withRouter } from "react-router-dom";
import Flatpickr from "react-flatpickr";
import { ChevronDownIcon } from '@heroicons/react/20/solid'
import { Transition } from '@headlessui/react'
import { ChevronDownIcon } from '@heroicons/react/20/solid';
import { Transition } from '@headlessui/react';
import {
shiftDays,
shiftMonths,
Expand All @@ -25,9 +25,9 @@
isSameDate
} from "./util/date";
import { navigateToQuery, QueryLink, QueryButton } from "./query";
import { shouldIgnoreKeypress } from "./keybinding.js"
import { COMPARISON_DISABLED_PERIODS, toggleComparisons, isComparisonEnabled } from "../dashboard/comparison-input.js"
import classNames from "classnames"
import { shouldIgnoreKeypress } from "./keybinding.js";
import { COMPARISON_DISABLED_PERIODS, toggleComparisons, isComparisonEnabled } from "../dashboard/comparison-input.js";
import classNames from "classnames";
import { useQueryContext } from "./query-context.js";
import { useSiteContext } from "./site-context.js";

Expand Down Expand Up @@ -65,7 +65,6 @@
<div className={containerClass}>
<QueryButton
to={{ date: prevDate }}
query={query}
className={leftClass}
disabled={disabledLeft}
>
Expand All @@ -84,7 +83,6 @@
</QueryButton>
<QueryButton
to={{ date: nextDate }}
query={query}
className={rightClass}
disabled={disabledRight}
>
Expand All @@ -105,7 +103,9 @@
);
}

function DatePickerArrows({ site, query }) {
function DatePickerArrows() {
const { query } = useQueryContext();
const site = useSiteContext();
if (query.period === "year") {
const prevDate = formatISO(shiftMonths(query.date, -12));
const nextDate = formatISO(shiftMonths(query.date, 12));
Expand All @@ -126,7 +126,9 @@
return null
}

function DisplayPeriod({ query, site }) {
function DisplayPeriod() {
const { query } = useQueryContext();
const site = useSiteContext();
if (query.period === "day") {
if (isToday(site, query.date)) {
return "Today";
Expand Down Expand Up @@ -237,7 +239,7 @@
} else if (newSearch.date) {
navigateToQuery(history, query, newSearch);
}
}, [query])

Check warning on line 242 in assets/js/dashboard/datepicker.js

View workflow job for this annotation

GitHub Actions / Build and test

React Hook useCallback has missing dependencies: 'history' and 'site'. Either include them or remove the dependency array

const handleClick = useCallback((e) => {
if (dropDownNode.current && dropDownNode.current.contains(e.target)) return;
Expand Down Expand Up @@ -302,7 +304,6 @@
<QueryLink
to={{ from: false, to: false, period, ...opts }}
onClick={() => setOpen(false)}
query={query}
className={`${boldClass} px-4 py-2 text-sm leading-tight hover:bg-gray-100 hover:text-gray-900
dark:hover:bg-gray-900 dark:hover:text-gray-100 flex items-center justify-between`}
>
Expand Down Expand Up @@ -414,7 +415,7 @@
aria-controls="datemenu"
>
<span className="truncate mr-1 md:mr-2">
<span className="font-medium"><DisplayPeriod query={query} site={site} /></span>
<span className="font-medium"><DisplayPeriod /></span>
</span>
<ChevronDownIcon className="hidden sm:inline-block h-4 w-4 md:h-5 md:w-5 text-gray-500" />
</div>
Expand All @@ -437,7 +438,7 @@

return (
<div className="flex ml-auto pl-2">
<DatePickerArrows site={site} query={query} />
<DatePickerArrows />
{renderPicker()}
</div>
)
Expand Down
34 changes: 19 additions & 15 deletions assets/js/dashboard/extra/funnel.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
import React, { useEffect, useState, useRef } from 'react'
import React, { useEffect, useState, useRef } from 'react';
import FlipMove from 'react-flip-move';
import Chart from 'chart.js/auto'
import FunnelTooltip from './funnel-tooltip.js'
import ChartDataLabels from 'chartjs-plugin-datalabels'
import numberFormatter from '../util/number-formatter'
import Bar from '../stats/bar'
import Chart from 'chart.js/auto';
import FunnelTooltip from './funnel-tooltip.js';
import ChartDataLabels from 'chartjs-plugin-datalabels';
import numberFormatter from '../util/number-formatter';
import Bar from '../stats/bar';

import RocketIcon from '../stats/modals/rocket-icon'
import RocketIcon from '../stats/modals/rocket-icon';

import * as api from '../api'
import LazyLoader from '../components/lazy-loader'
import * as api from '../api';
import LazyLoader from '../components/lazy-loader';
import { useQueryContext } from '../query-context.js';
import { useSiteContext } from '../site-context.js';


export default function Funnel(props) {
export default function Funnel({ funnelName, tabs }) {
const site = useSiteContext();
const { query } = useQueryContext();
const [loading, setLoading] = useState(true)
const [visible, setVisible] = useState(false)
const [error, setError] = useState(undefined)
Expand Down Expand Up @@ -42,7 +46,7 @@ export default function Funnel(props) {
}
}
}
}, [props.query, props.funnelName, visible, isSmallScreen])
}, [query, funnelName, visible, isSmallScreen])

useEffect(() => {
if (canvasRef.current && funnel && visible && !isSmallScreen) {
Expand Down Expand Up @@ -115,15 +119,15 @@ export default function Funnel(props) {
}

const getFunnel = () => {
return props.site.funnels.find((funnel) => funnel.name === props.funnelName)
return site.funnels.find((funnel) => funnel.name === funnelName)
}

const fetchFunnel = async () => {
const funnelMeta = getFunnel()
if (typeof funnelMeta === 'undefined') {
throw new Error('Could not fetch the funnel. Perhaps it was deleted?')
} else {
return api.get(`/api/stats/${encodeURIComponent(props.site.domain)}/funnels/${funnelMeta.id}`, props.query)
return api.get(`/api/stats/${encodeURIComponent(site.domain)}/funnels/${funnelMeta.id}`, query)
}
}

Expand Down Expand Up @@ -256,8 +260,8 @@ export default function Funnel(props) {
const header = () => {
return (
<div className="flex justify-between w-full">
<h4 className="mt-2 text-sm dark:text-gray-100">{props.funnelName}</h4>
{props.tabs}
<h4 className="mt-2 text-sm dark:text-gray-100">{funnelName}</h4>
{tabs}
</div>
)
}
Expand Down
19 changes: 10 additions & 9 deletions assets/js/dashboard/filters.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React, { Fragment, useEffect, useState } from 'react';
import { Link, withRouter } from 'react-router-dom'
import { AdjustmentsVerticalIcon, MagnifyingGlassIcon, XMarkIcon, PencilSquareIcon } from '@heroicons/react/20/solid'
import classNames from 'classnames'
import { Menu, Transition } from '@headlessui/react'
import { Link, withRouter } from 'react-router-dom';
import { AdjustmentsVerticalIcon, MagnifyingGlassIcon, XMarkIcon, PencilSquareIcon } from '@heroicons/react/20/solid';
import classNames from 'classnames';
import { Menu, Transition } from '@headlessui/react';

import { navigateToQuery } from './query'
import { navigateToQuery } from './query';
import {
FILTER_GROUP_TO_MODAL_TYPE,
cleanLabels,
Expand All @@ -15,7 +15,7 @@ import {
getPropertyKeyFromFilterKey,
getLabel,
FILTER_OPERATIONS_DISPLAY_NAMES
} from "./util/filters"
} from "./util/filters";
import { useQueryContext } from './query-context';
import { useSiteContext } from './site-context';

Expand Down Expand Up @@ -99,7 +99,9 @@ function filterDropdownOption(site, option) {
)
}

function DropdownContent({ history, site, query, wrapped }) {
function DropdownContent({ history, wrapped }) {
const site = useSiteContext();
const { query } = useQueryContext();
const [addingFilter, setAddingFilter] = useState(false);

if (wrapped === WRAPSTATE.unwrapped || addingFilter) {
Expand All @@ -126,7 +128,6 @@ function DropdownContent({ history, site, query, wrapped }) {

function Filters({ history }) {
const { query } = useQueryContext();
const site = useSiteContext();

const [wrapped, setWrapped] = useState(WRAPSTATE.waiting)
const [viewport, setViewport] = useState(1080)
Expand Down Expand Up @@ -267,7 +268,7 @@ function Filters({ history }) {
className="rounded-md shadow-lg bg-white dark:bg-gray-800 ring-1 ring-black ring-opacity-5
font-medium text-gray-800 dark:text-gray-200"
>
<DropdownContent history={history} query={query} site={site} wrapped={wrapped} />
<DropdownContent history={history} wrapped={wrapped} />
</div>
</Menu.Items>
</Transition>
Expand Down
3 changes: 2 additions & 1 deletion assets/js/dashboard/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import React, { useState } from 'react';

import Historical from './historical'
import Realtime, { useIsRealtimeDashboard } from './realtime'
import Realtime from './realtime'
import { useIsRealtimeDashboard } from './util/filters'

export const statsBoxClass = "stats-item relative w-full mt-6 p-4 flex flex-col bg-white dark:bg-gray-825 shadow-xl rounded"

Expand Down
15 changes: 9 additions & 6 deletions assets/js/dashboard/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import * as storage from './util/storage'
import { COMPARISON_DISABLED_PERIODS, getStoredComparisonMode, isComparisonEnabled, getStoredMatchDayOfWeek } from './comparison-input'
import { getFiltersByKeyPrefix } from './util/filters'

import dayjs from 'dayjs';
import utc from 'dayjs/plugin/utc';
import dayjs from 'dayjs'
import utc from 'dayjs/plugin/utc'
import { parseLegacyFilter, parseLegacyPropsFilter } from './util/filters'
import { useQueryContext } from './query-context'

dayjs.extend(utc)

Expand Down Expand Up @@ -49,7 +50,7 @@ export function parseQuery(querystring, site) {
}

export function addFilter(query, filter) {
return {...query, filters: [...query.filters, filter]}
return { ...query, filters: [...query.filters, filter] }
}

export function navigateToQuery(history, queryFrom, newData) {
Expand Down Expand Up @@ -145,7 +146,7 @@ export function filtersBackwardsCompatibilityRedirect() {
export function revenueAvailable(query, site) {
const revenueGoalsInFilter = site.revenueGoals.filter((rg) => {
const goalFilters = getFiltersByKeyPrefix(query, "goal")

return goalFilters.some(([_op, _key, clauses]) => {
return clauses.includes(rg.event_name)
})
Expand All @@ -159,7 +160,8 @@ export function revenueAvailable(query, site) {
}

function QueryLink(props) {
const { query, history, to, className, children } = props
const { query } = useQueryContext();
const { history, to, className, children } = props

function onClick(e) {
e.preventDefault()
Expand All @@ -183,7 +185,8 @@ function QueryLink(props) {
const QueryLinkWithRouter = withRouter(QueryLink)
export { QueryLinkWithRouter as QueryLink };

function QueryButton({ history, query, to, disabled, className, children, onClick }) {
function QueryButton({ history, to, disabled, className, children, onClick }) {
const { query } = useQueryContext();
return (
<button
className={className}
Expand Down
9 changes: 1 addition & 8 deletions assets/js/dashboard/realtime.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React, { useMemo } from 'react';

import React from 'react';
import Datepicker from './datepicker'
import SiteSwitcher from './site-switcher'
import Filters from './filters'
Expand All @@ -14,12 +13,6 @@ import { statsBoxClass } from './index';
import { useSiteContext } from './site-context';
import { useUserContext } from './user-context';
import { useQueryContext } from './query-context';
import { isRealTimeDashboard } from './util/filters';

export const useIsRealtimeDashboard = () => {
const { query: { period } } = useQueryContext();
return useMemo(() => isRealTimeDashboard({ period }), [period]);
}

function Realtime({ stuck }) {
const site = useSiteContext();
Expand Down
22 changes: 12 additions & 10 deletions assets/js/dashboard/stats/behaviours/conversions.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import React from 'react';
import * as api from '../../api'
import * as url from '../../util/url'
import * as api from '../../api';
import * as url from '../../util/url';

import * as metrics from '../reports/metrics';
import ListReport from '../reports/list';
import { useSiteContext } from '../../site-context';
import { useQueryContext } from '../../query-context';

export default function Conversions(props) {
const { site, query, afterFetchData } = props
export default function Conversions({ afterFetchData, onGoalFilterClick }) {
const site = useSiteContext();
const { query } = useQueryContext()

function fetchConversions() {
return api.get(url.apiPath(site, '/conversions'), query, { limit: 9 })
Expand All @@ -21,11 +24,11 @@ export default function Conversions(props) {

function chooseMetrics() {
return [
metrics.createVisitors({ renderLabel: (_query) => "Uniques", meta: {plot: true}}),
metrics.createEvents({renderLabel: (_query) => "Total", meta: {hiddenOnMobile: true}}),
metrics.createVisitors({ renderLabel: (_query) => "Uniques", meta: { plot: true } }),
metrics.createEvents({ renderLabel: (_query) => "Total", meta: { hiddenOnMobile: true } }),
metrics.createConversionRate(),
BUILD_EXTRA && metrics.createTotalRevenue({meta: {hiddenOnMobile: true}}),
BUILD_EXTRA && metrics.createAverageRevenue({meta: {hiddenOnMobile: true}})
BUILD_EXTRA && metrics.createTotalRevenue({ meta: { hiddenOnMobile: true } }),
BUILD_EXTRA && metrics.createAverageRevenue({ meta: { hiddenOnMobile: true } })
].filter(metric => !!metric)
}

Expand All @@ -36,11 +39,10 @@ export default function Conversions(props) {
afterFetchData={afterFetchData}
getFilterFor={getFilterFor}
keyLabel="Goal"
onClick={props.onGoalFilterClick}
onClick={onGoalFilterClick}
metrics={chooseMetrics()}
detailsLink={url.sitePath('conversions')}
maybeHideDetails={true}
query={query}
color="bg-red-50"
colMinWidth={90}
/>
Expand Down
Loading
Loading