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

feat(ui): Allow specifying whether local or all snapshots are shown by default #186

Merged
merged 1 commit into from
Sep 14, 2023
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
16 changes: 12 additions & 4 deletions src/SourcesTable.jsx
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @PhracturedBlue,

there is a lint error in line 222:3. Can you fix it? Additionally, there is console log within the code that should be removed.

Right now we need to override the selectedOwner in the rendering function, which basically throws the above mentioned lint error. For the first rendering, the state is not changed. The second rendering cycle then uses the set selectedOwner.

Can we fix this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM

Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class SourcesTable extends Component {

localSourceName: "",
multiUser: false,
selectedOwner: localSnapshots,
selectedOwner: null,
selectedDirectory: "",
};

Expand Down Expand Up @@ -79,9 +79,15 @@ export class SourcesTable extends Component {
}

selectOwner(h) {
const { setDefaultSnapshotViewAll } = this.context;
this.setState({
selectedOwner: h,
});
if (h === localSnapshots) {
setDefaultSnapshotViewAll(false);
} else if (h === allSnapshots) {
setDefaultSnapshotViewAll(true);
}
}

sync() {
Expand Down Expand Up @@ -211,14 +217,16 @@ export class SourcesTable extends Component {

render() {
let { sources, isLoading, error } = this.state;
const { bytesStringBase2 } = this.context
const { bytesStringBase2, defaultSnapshotViewAll } = this.context
if (error) {
return <p>{error.message}</p>;
}
if (isLoading) {
return <Spinner animation="border" variant="primary" />;
}

if (this.state.selectedOwner == null) {
this.setState({ selectedOwner: defaultSnapshotViewAll ? allSnapshots : localSnapshots})
}
let uniqueOwners = sources.reduce((a, d) => {
const owner = ownerName(d.source);

Expand Down Expand Up @@ -324,4 +332,4 @@ export class SourcesTable extends Component {
</>;
}
}
SourcesTable.contextType = UIPreferencesContext
SourcesTable.contextType = UIPreferencesContext
12 changes: 10 additions & 2 deletions src/contexts/UIPreferencesContext.tsx
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should extend the interface SerializedUIPreferences to reflect the new attribute defaultSnapshotViewAll.

Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import axios from 'axios';
export const PAGE_SIZES = [10, 20, 30, 40, 50, 100];
export const UIPreferencesContext = React.createContext<UIPreferences>({} as UIPreferences);

const DEFAULT_PREFERENCES = { pageSize: PAGE_SIZES[0], bytesStringBase2: false, theme: getDefaultTheme() } as SerializedUIPreferences;
const DEFAULT_PREFERENCES = { pageSize: PAGE_SIZES[0], bytesStringBase2: false, defaultSnapshotViewAll: false, theme: getDefaultTheme() } as SerializedUIPreferences;
const PREFERENCES_URL = '/api/v1/ui-preferences';

export type Theme = "light" | "dark" | "pastel" | "ocean";
Expand All @@ -14,14 +14,17 @@ export interface UIPreferences {
get pageSize(): PageSize
get theme(): Theme
get bytesStringBase2(): boolean
get defaultSnapshotViewAll(): boolean
setTheme: (theme: Theme) => void
setPageSize: (pageSize: number) => void
setByteStringBase: (bytesStringBase2: String) => void
setDefaultSnapshotViewAll: (defaultSnapshotView: boolean) => void
}

interface SerializedUIPreferences {
pageSize?: number
bytesStringBase2?: boolean
defaultSnapshotView?: boolean
theme: Theme | undefined
}

Expand Down Expand Up @@ -114,7 +117,12 @@ export function UIPreferenceProvider(props: UIPreferenceProviderProps) {
return { ...oldPreferences, bytesStringBase2 };
});

const providedValue = { ...preferences, setTheme, setPageSize, setByteStringBase} as UIPreferences;
const setDefaultSnapshotViewAll = (input: boolean) => setPreferences(oldPreferences => {
var defaultSnapshotViewAll = input;
return { ...oldPreferences, defaultSnapshotViewAll };
});

const providedValue = { ...preferences, setTheme, setPageSize, setByteStringBase, setDefaultSnapshotViewAll} as UIPreferences;

return <UIPreferencesContext.Provider value={providedValue}>
{props.children}
Expand Down
Loading