Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

Fix app selector scrolling #858

Merged
merged 4 commits into from
Jul 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
8 changes: 0 additions & 8 deletions public/app/components/AppSelector/AppSelector.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@
max-width: calc(100vw - 290px);
}

.search {
background: var(--ps-immutable-off-white);
color: var(--ps-immutable-placeholder-text);
border: 1px solid var(--ps-ui-border);
margin: 10px 0 10px 10px;
border-radius: 4px;
}

.noData {
width: 600px;
height: 300px;
Expand Down
34 changes: 32 additions & 2 deletions public/app/components/AppSelector/AppSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,36 @@ export const SelectorModalWithToggler = ({
return selectedApp?.__profile_type__ === a.__profile_type__;
};

const groups = useMemo(() => {
const allGroups = leftSideApps.map((app) => app.name.split('-')[0]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess here we should maybe use service_name and simply calculate the height of the dropdown based off the number of service_name.

If I'm understanding correctly the idea here is to:

  1. Calculate the necessary height of the modal based off the number of "groups" that will exist
  2. If the modal is bigger than the screen then set the modal to slightly bigger height than the screen... if the modal is smaller than the screen then just adapt to the size of the modal

right now the logic is assuming that the "groups" are differentiated by the text before the first - but thats not true. I guess they are now based off of the service_name?

cc @korniltsev @cyriltovena @petethepig

Copy link
Contributor Author

@Rperry2174 Rperry2174 Jul 13, 2023

Choose a reason for hiding this comment

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

image

this isn't super helpful because I just cheated so I could make the dropdown very high by adding random numbers to each name, but for production data, for example, will the height of the modal will be determined by the unique number of service_names

const uniqGroups = Array.from(new Set(allGroups));

const dedupedUniqGroups = uniqGroups.filter((x) => {
return !uniqGroups.find((y) => x !== y && y.startsWith(x));
});

const groupOrApp = dedupedUniqGroups.map((groupName) => {
const appNamesEntries = leftSideApps.filter((app) =>
app.name.startsWith(groupName)
);

return appNamesEntries.length > 1 ? groupName : appNamesEntries[0].name;
});

return groupOrApp;
}, [leftSideApps]);

const listHeight = useMemo(() => {
const windowHeight = window?.innerHeight || 0;
const listRequiredHeight = Math.max(groups.length, matchedApps.length) * 35;

if (windowHeight && listRequiredHeight) {
return windowHeight >= listRequiredHeight ? 'auto' : `${windowHeight}px`;
}

return 'auto';
}, [groups, matchedApps]);

return (
<ModalWithToggle
isModalOpen={isModalOpen}
Expand All @@ -143,7 +173,7 @@ export const SelectorModalWithToggler = ({
setSelectedLeftSide(undefined);
setModalOpenStatus(false);
}}
modalHeight={'auto'}
modalHeight={listHeight}
noDataEl={
!leftSideApps?.length ? (
<div data-testid="app-selector-no-data" className={ogStyles.noData}>
Expand All @@ -164,7 +194,7 @@ export const SelectorModalWithToggler = ({
placeholder="Search..."
value={filter}
onChange={(e) => setFilter(e.target.value)}
className={styles.search}
className={ogStyles.search}
data-testid="app-selector-search"
/>
</>
Expand Down