Skip to content

Commit

Permalink
chore: Refactor localstorage into typesafe version (apache#17832)
Browse files Browse the repository at this point in the history
  • Loading branch information
Erik Ritter authored and bwang221 committed Feb 10, 2022
1 parent 4332b7f commit 0fcacbb
Show file tree
Hide file tree
Showing 14 changed files with 202 additions and 98 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -237,14 +237,14 @@ const config: ControlPanelConfig = {
],
renderTrigger: true,
description: (
<React.Fragment>
<>
<div>{t('Change order of rows.')}</div>
<div>{t('Available sorting modes:')}</div>
<ul>
<li>{t('By key: use row names as sorting key')}</li>
<li>{t('By value: use metric values as sorting key')}</li>
</ul>
</React.Fragment>
</>
),
},
},
Expand All @@ -265,14 +265,14 @@ const config: ControlPanelConfig = {
],
renderTrigger: true,
description: (
<React.Fragment>
<>
<div>{t('Change order of columns.')}</div>
<div>{t('Available sorting modes:')}</div>
<ul>
<li>{t('By key: use column names as sorting key')}</li>
<li>{t('By value: use metric values as sorting key')}</li>
</ul>
</React.Fragment>
</>
),
},
},
Expand Down
20 changes: 12 additions & 8 deletions superset-frontend/src/dashboard/containers/DashboardPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import { addWarningToast } from 'src/components/MessageToasts/actions';

import {
getFromLocalStorage,
setInLocalStorage,
getItem,
setItem,
LocalStorageKeys,
} from 'src/utils/localStorageHelpers';
import {
FILTER_BOX_MIGRATION_STATES,
FILTER_BOX_TRANSITION_SNOOZE_DURATION,
FILTER_BOX_TRANSITION_SNOOZED_AT,
} from 'src/explore/constants';
import { URL_PARAMS } from 'src/constants';
import { getUrlParam } from 'src/utils/urlUtils';
Expand Down Expand Up @@ -126,8 +126,10 @@ const DashboardPage: FC = () => {
}

// has cookie?
const snoozeDash =
getFromLocalStorage(FILTER_BOX_TRANSITION_SNOOZED_AT, 0) || {};
const snoozeDash = getItem(
LocalStorageKeys.filter_box_transition_snoozed_at,
{},
);
if (
Date.now() - (snoozeDash[id] || 0) <
FILTER_BOX_TRANSITION_SNOOZE_DURATION
Expand Down Expand Up @@ -210,9 +212,11 @@ const DashboardPage: FC = () => {
setFilterboxMigrationState(FILTER_BOX_MIGRATION_STATES.REVIEWING);
}}
onClickSnooze={() => {
const snoozedDash =
getFromLocalStorage(FILTER_BOX_TRANSITION_SNOOZED_AT, 0) || {};
setInLocalStorage(FILTER_BOX_TRANSITION_SNOOZED_AT, {
const snoozedDash = getItem(
LocalStorageKeys.filter_box_transition_snoozed_at,
{},
);
setItem(LocalStorageKeys.filter_box_transition_snoozed_at, {
...snoozedDash,
[id]: Date.now(),
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ import TableView, { EmptyWrapperType } from 'src/components/TableView';
import { getChartDataRequest } from 'src/chart/chartAction';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import {
getFromLocalStorage,
setInLocalStorage,
getItem,
setItem,
LocalStorageKeys,
} from 'src/utils/localStorageHelpers';
import {
CopyToClipboardButton,
Expand All @@ -49,10 +50,6 @@ const NULLISH_RESULTS_STATE = {

const DATA_TABLE_PAGE_SIZE = 50;

const STORAGE_KEYS = {
isOpen: 'is_datapanel_open',
};

const DATAPANEL_KEY = 'data';

const TableControlsWrapper = styled.div`
Expand Down Expand Up @@ -200,7 +197,7 @@ export const DataTablesPane = ({
[RESULT_TYPES.samples]?: boolean;
}>(NULLISH_RESULTS_STATE);
const [panelOpen, setPanelOpen] = useState(
getFromLocalStorage(STORAGE_KEYS.isOpen, false),
getItem(LocalStorageKeys.is_datapanel_open, false),
);

const formattedData = useMemo(
Expand Down Expand Up @@ -283,7 +280,7 @@ export const DataTablesPane = ({
);

useEffect(() => {
setInLocalStorage(STORAGE_KEYS.isOpen, panelOpen);
setItem(LocalStorageKeys.is_datapanel_open, panelOpen);
}, [panelOpen]);

useEffect(() => {
Expand Down
13 changes: 5 additions & 8 deletions superset-frontend/src/explore/components/ExploreChartPanel.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ import { useResizeDetector } from 'react-resize-detector';
import { chartPropShape } from 'src/dashboard/util/propShapes';
import ChartContainer from 'src/chart/ChartContainer';
import {
getFromLocalStorage,
setInLocalStorage,
getItem,
setItem,
LocalStorageKeys,
} from 'src/utils/localStorageHelpers';
import ConnectedExploreChartHeader from './ExploreChartHeader';
import { DataTablesPane } from './DataTablesPane';
Expand Down Expand Up @@ -64,10 +65,6 @@ const CHART_PANEL_PADDING_HORIZ = 30;
const CHART_PANEL_PADDING_VERTICAL = 15;
const HEADER_PADDING = 15;

const STORAGE_KEYS = {
sizes: 'chart_split_sizes',
};

const INITIAL_SIZES = [90, 10];
const MIN_SIZES = [300, 50];
const DEFAULT_SOUTH_PANE_HEIGHT_PERCENT = 40;
Expand Down Expand Up @@ -126,7 +123,7 @@ const ExploreChartPanel = props => {
refreshRate: 300,
});
const [splitSizes, setSplitSizes] = useState(
getFromLocalStorage(STORAGE_KEYS.sizes, INITIAL_SIZES),
getItem(LocalStorageKeys.chart_split_sizes, INITIAL_SIZES),
);
const { slice } = props;
const updateQueryContext = useCallback(
Expand Down Expand Up @@ -192,7 +189,7 @@ const ExploreChartPanel = props => {
}, [recalcPanelSizes, splitSizes]);

useEffect(() => {
setInLocalStorage(STORAGE_KEYS.sizes, splitSizes);
setItem(LocalStorageKeys.chart_split_sizes, splitSizes);
}, [splitSizes]);

const onDragEnd = sizes => {
Expand Down
26 changes: 11 additions & 15 deletions superset-frontend/src/explore/components/ExploreViewContainer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ import { usePrevious } from 'src/common/hooks/usePrevious';
import { useComponentDidMount } from 'src/common/hooks/useComponentDidMount';
import Icons from 'src/components/Icons';
import {
getFromLocalStorage,
setInLocalStorage,
getItem,
setItem,
LocalStorageKeys,
} from 'src/utils/localStorageHelpers';
import { URL_PARAMS } from 'src/constants';
import cx from 'classnames';
Expand Down Expand Up @@ -183,11 +184,6 @@ function ExploreViewContainer(props) {
? `${props.forcedHeight}px`
: `${windowSize.height - navHeight}px`;

const storageKeys = {
controlsWidth: 'controls_width',
dataSourceWidth: 'datasource_width',
};

const defaultSidebarsWidth = {
controls_width: 320,
datasource_width: 300,
Expand Down Expand Up @@ -455,12 +451,12 @@ function ExploreViewContainer(props) {
}

function getSidebarWidths(key) {
return getFromLocalStorage(key, defaultSidebarsWidth[key]);
return getItem(key, defaultSidebarsWidth[key]);
}

function setSidebarWidths(key, dimension) {
const newDimension = Number(getSidebarWidths(key)) + dimension.width;
setInLocalStorage(key, newDimension);
setItem(key, newDimension);
}

if (props.standalone) {
Expand Down Expand Up @@ -504,13 +500,13 @@ function ExploreViewContainer(props) {
)}
<Resizable
onResizeStop={(evt, direction, ref, d) =>
setSidebarWidths(storageKeys.dataSourceWidth, d)
setSidebarWidths(LocalStorageKeys.datasource_width, d)
}
defaultSize={{
width: getSidebarWidths(storageKeys.dataSourceWidth),
width: getSidebarWidths(LocalStorageKeys.datasource_width),
height: '100%',
}}
minWidth={defaultSidebarsWidth[storageKeys.dataSourceWidth]}
minWidth={defaultSidebarsWidth[LocalStorageKeys.datasource_width]}
maxWidth="33%"
enable={{ right: true }}
className={
Expand Down Expand Up @@ -564,13 +560,13 @@ function ExploreViewContainer(props) {
) : null}
<Resizable
onResizeStop={(evt, direction, ref, d) =>
setSidebarWidths(storageKeys.controlsWidth, d)
setSidebarWidths(LocalStorageKeys.controls_width, d)
}
defaultSize={{
width: getSidebarWidths(storageKeys.controlsWidth),
width: getSidebarWidths(LocalStorageKeys.controls_width),
height: '100%',
}}
minWidth={defaultSidebarsWidth[storageKeys.controlsWidth]}
minWidth={defaultSidebarsWidth[LocalStorageKeys.controls_width]}
maxWidth="33%"
enable={{ right: true }}
className="col-sm-3 explore-column controls-column"
Expand Down
2 changes: 0 additions & 2 deletions superset-frontend/src/explore/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,6 @@ export enum FILTER_BOX_MIGRATION_STATES {
UNDECIDED = 'UNDECIDED',
}

export const FILTER_BOX_TRANSITION_SNOOZED_AT =
'filter_box_transition_snoozed_at';
export const FILTER_BOX_TRANSITION_SNOOZE_DURATION = 24 * 60 * 60 * 1000; // 24 hours

export const POPOVER_INITIAL_HEIGHT = 240;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,28 @@
* specific language governing permissions and limitations
* under the License.
*/
import {
getItem,
setItem,
LocalStorageKeys,
} from 'src/utils/localStorageHelpers';

// storage keys for welcome page sticky tabs and tables
export const HOMEPAGE_CHART_FILTER = 'homepage_chart_filter';
export const HOMEPAGE_ACTIVITY_FILTER = 'homepage_activity_filter';
export const HOMEPAGE_DASHBOARD_FILTER = 'homepage_dashboard_filter';
export const HOMEPAGE_COLLAPSE_STATE = 'homepage_collapse_state';
describe('localStorageHelpers', () => {
beforeEach(() => {
localStorage.clear();
});

afterAll(() => {
localStorage.clear();
});

it('gets a value that was set', () => {
setItem(LocalStorageKeys.is_datapanel_open, false);

expect(getItem(LocalStorageKeys.is_datapanel_open, true)).toBe(false);
});

it('returns the default value for an unset value', () => {
expect(getItem(LocalStorageKeys.is_datapanel_open, true)).toBe(true);
});
});
89 changes: 84 additions & 5 deletions superset-frontend/src/utils/localStorageHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,98 @@
* under the License.
*/

export const getFromLocalStorage = (key: string, defaultValue: any) => {
import { TableTabTypes } from 'src/views/CRUD/types';
import { SetTabType } from 'src/views/CRUD/welcome/ActivityTable';

export enum LocalStorageKeys {
/**
* START LEGACY LOCAL STORAGE KEYS
*
* Do not follow the patterns here for key names. Keys should instead be namespaced to avoid
* collisions.
*
* TODO: Update all local storage keys to follow the new pattern. This is a breaking change,
* and therefore should be done in a major release.
*/
filter_box_transition_snoozed_at = 'filter_box_transition_snoozed_at',
chart_split_sizes = 'chart_split_sizes',
controls_width = 'controls_width',
datasource_width = 'datasource_width',
is_datapanel_open = 'is_datapanel_open',
homepage_chart_filter = 'homepage_chart_filter',
homepage_dashboard_filter = 'homepage_dashboard_filter',
homepage_collapse_state = 'homepage_collapse_state',
homepage_activity_filter = 'homepage_activity_filter',
/** END LEGACY LOCAL STORAGE KEYS */

/**
* New local storage keys should be namespaced to avoid collisions. Keys should be named in the
* form [namespace]__[key].
*
* Example:
* sqllab__is_autocomplete_enabled
*/
}

export type LocalStorageValues = {
filter_box_transition_snoozed_at: Record<number, number>;
chart_split_sizes: [number, number];
controls_width: number;
datasource_width: number;
is_datapanel_open: boolean;
homepage_chart_filter: TableTabTypes;
homepage_dashboard_filter: TableTabTypes;
homepage_collapse_state: string[];
homepage_activity_filter: SetTabType | null;
};

export function getItem<K extends LocalStorageKeys>(
key: K,
defaultValue: LocalStorageValues[K],
): LocalStorageValues[K] {
return dangerouslyGetItemDoNotUse(key, defaultValue);
}

export function setItem<K extends LocalStorageKeys>(
key: K,
value: LocalStorageValues[K],
): void {
dangerouslySetItemDoNotUse(key, value);
}

/*
* This function should not be used directly, as it doesn't provide any type safety or any
* guarantees that the globally namespaced localstorage key is correct.
*
* Instead, use getItem and setItem. Any legacy uses should be updated/migrated in future
* Superset versions (as they may require breaking changes).
* */
export function dangerouslyGetItemDoNotUse(
key: string,
defaultValue: any,
): any {
try {
const value = localStorage.getItem(key);
return JSON.parse(value || 'null') || defaultValue;
if (value === null) {
return defaultValue;
}
return JSON.parse(value);
} catch {
return defaultValue;
}
};
}

export const setInLocalStorage = (key: string, value: any) => {
/*
* This function should not be used directly, as it doesn't provide any type safety or any
* guarantees that the globally namespaced localstorage key is correct.
*
* Instead, use getItem and setItem. Any legacy uses should be updated/migrated in future
* Superset versions (as they may require breaking changes).
* */
export function dangerouslySetItemDoNotUse(key: string, value: any): void {
try {
localStorage.setItem(key, JSON.stringify(value));
} catch {
// Catch in case localStorage is unavailable
}
};
}

0 comments on commit 0fcacbb

Please sign in to comment.