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(query-bar): allow users to set the default sort to be recent first COMPASS-6706 #6663

Merged
merged 24 commits into from
Feb 4, 2025
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
c
djechlin-mongodb committed Jan 30, 2025
commit 72f06975df72ff3a79cdfafd8781a936d4d6f8b7
14 changes: 14 additions & 0 deletions packages/compass-preferences-model/src/preferences-schema.ts
Original file line number Diff line number Diff line change
@@ -1193,3 +1193,17 @@ export function getSettingDescription<
type,
};
}

export function getSettingSelectableValues<
Name extends Exclude<keyof AllPreferences, keyof InternalUserPreferences>
>(
name: Name
): Pick<PreferenceDefinition<Name>, 'selectableValues'> & { type: unknown } {
const { selectableValues, type } = allPreferencesProps[
name
] as PreferenceDefinition<Name>;
return {
selectableValues,
type,
};
}
5 changes: 4 additions & 1 deletion packages/compass-preferences-model/src/provider.ts
Original file line number Diff line number Diff line change
@@ -10,6 +10,9 @@ export {
} from './utils';
export { capMaxTimeMSAtPreferenceLimit } from './maxtimems';
export { featureFlags } from './feature-flags';
export { getSettingDescription } from './preferences-schema';
export {
getSettingDescription,
getSettingSelectableValues,
} from './preferences-schema';
export type { AllPreferences } from './preferences-schema';
export type { DevtoolsProxyOptions } from '@mongodb-js/devtools-proxy-support';
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@ import React, { useCallback } from 'react';
import type { UserConfigurablePreferences } from 'compass-preferences-model';
import {
getSettingDescription,
getSettingSelectableValues,
featureFlags,
} from 'compass-preferences-model/provider';
import { settingStateLabels } from './state-labels';
@@ -18,6 +19,7 @@ import {
import { changeFieldValue } from '../../stores/settings';
import type { RootState } from '../../stores';
import { connect } from 'react-redux';
import { get } from 'lodash';

type KeysMatching<T, V> = keyof {
[P in keyof T as T[P] extends V ? P : never]: P;
@@ -170,6 +172,7 @@ function DropdownSetting<PreferenceName extends StringPreferences>({
value: string | undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think undefined is ever possible here and you're getting a compilation error that's related to that: leafygreen Select tries to do some infers to smartly detect uncontrolled component and shows some optional props as required (you can see those errors in CI):

@mongodb-js/compass-settings: src/components/settings/settings-list.tsx(185,8): error TS2322: Type '{ children: Element[]; className: string; allowDeselect: false; "aria-labelledby": string; id: PreferenceName; name: PreferenceName; "data-testid": PreferenceName; value: string | undefined; onChange: (value: string) => void; disabled: boolean; }' is not assignable to type 'IntrinsicAttributes & ((Omit<BaseSelectProps & Omit<LabelProp, "aria-label" | "aria-labelledby" | "label"> & Required<...> & Partial<...> & { ...; } & { ...; }, "ref"> | ... 7 more ... | Omit<...>) & RefAttributes<...>)'.
@mongodb-js/compass-settings:   Type '{ children: Element[]; className: string; allowDeselect: false; "aria-labelledby": string; id: PreferenceName; name: PreferenceName; "data-testid": PreferenceName; value: string | undefined; onChange: (value: string) => void; disabled: boolean; }' is missing the following properties from type 'Omit<BaseSelectProps & Omit<LabelProp, "aria-label" | "aria-labelledby" | "label"> & Required<Pick<LabelProp, "label">> & Partial<...> & { ...; } & { ...; }, "ref">': label, readOnly

Generally speaking controlled react component should always get the value, so this error is legit, it just shows itself in a weird way at the moment. If somehow we expect the value here to not always be provided it needs to be handled when component is rendered

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

disabled: boolean;
}) {
selectableValues = JSON.parse(JSON.stringify(selectableValues));
const onChangeEvent = useCallback(
(event: React.ChangeEvent<HTMLSelectElement>) => {
onChange(
@@ -301,7 +304,7 @@ function SettingsInput({

const { name, type, onChange, value, selectableValues } = props;

console.log('will we match');
console.log('will we match, ', type);
if (type === 'boolean') {
input = (
<BooleanSetting
@@ -312,7 +315,7 @@ function SettingsInput({
/>
);
} else if (type === 'string' && selectableValues) {
console.log('yay we matched');
console.log('yay we matched ', selectableValues);
input = (
<DropdownSetting
name={name}
@@ -362,11 +365,9 @@ const ConnectedSettingsInput = connect(
const { name } = ownProps;
const { type } = getSettingDescription(name);

console.log('settings: ', settings);
console.log('preferenceStates: ', preferenceStates);
return {
value: settings[name],
selectableValues: preferenceStates[name]?.selectableValues,
selectableValues: getSettingSelectableValues(name).selectableValues,
type: type,
disabled: !!preferenceStates[name],
stateLabel: settingStateLabels[preferenceStates[name] ?? ''],