Feat Add search input to chain, bridges and exchanges pages #305

DNR500 · 2024-09-24T11:27:57Z

This PR adds stick search inputs to the chains, bridges and exchanges pages

It reuses and makes a component out of the search input thats already in use for token selection and creates a set of reusable components across the pages.

There were a couple of caveats around the different layout height modes

When the layout is in default or max height mode the page could potentially collapse whilst a users was searching and this looked odd so the useFullPageInMaxHeightLayout hook was made with the intention ensuring the page takes up the maximum amount of space available.
Sticky mode when the Widget is in Full height layout mode requires some thought to ensure that it displays correctly. It also needs access to the header height, which it does using the useHeaderHeight hook.
- Whilst working on this I noticed that useHeaderHeight wasn't working properly I've created an alternative implementation here that use MutationObserver as a potential work around. I've some other ideas for potentially improvements in relation to this that I'll mention on the PR too.

There needed to be some thought around the checkbox functionality as well on the Bridges and Exchanges pages - mainly around the select all and deselect all checkbox. It would be good to check that if the behaviour that I have put together here is desirable.

DNR500 · 2024-09-24T18:16:31Z

packages/widget/src/hooks/useHeaderHeight.ts

+        resizeObserver.disconnect();
+      }
+    };
+  }, [elementId, headerHeight, headerElement, setHeaderHeight]);


Using the MutationObserver here in place of the imperative if checks we were doing previously - that was getting more difficult to get to get it to work in certain situations.

For the split subvariant there was still this problem in full height layout mode. And this problem can happen on other pages.

One improvement I think that would still be good here is to move the useLayoutEffect to the Header component itself and have that set a value on the Header Store - the useHeaderHeight hook would then become an access point for the height value in the store and using zustand shallow checks would actually be able to limit potential renders in the Widget where the height value it used

@DNR500 are we sure we want to see headerHeight in the deps array?

One improvement I think that would still be good here is to move the useLayoutEffect to the Header component itself and have that set a value on the Header Store - the useHeaderHeight hook would then become an access point for the height value in the store and using zustand shallow checks would actually be able to limit potential renders in the Widget where the height value it used

Should we do it now? 👀

I'm doing this move now and the deps array is much simpler and doesn't need the headerHeight.

…pages

github-actions · 2024-09-24T18:23:04Z

Hey! This is your new endopint: https://67ac0ba3.widget-addsearchi.pages.dev

chybisov · 2024-09-25T09:06:42Z

packages/widget/src/hooks/useHeaderHeight.ts

+        resizeObserver.disconnect();
+      }
+    };
+  }, [elementId, headerHeight, headerElement, setHeaderHeight]);


@DNR500 are we sure we want to see headerHeight in the deps array?

chybisov · 2024-09-25T09:07:34Z

packages/widget/src/hooks/useFullPageInMaxHeightLayout.ts

+// When the Widgets layout is set to use max height or in its default height mode the default behaviour
+// is for pages to use only the minimum height that they need to show their content.
+// On some pages you might want to always ensure that the page always up the maximum height available.
+// To do this just add this hook at the page level.
+// The other height layout will by default always occupy the maximum space available.
+// NOTE: this hook is implicitly tied to the widget height functionality in the
+//   AppExpandedContainer, RelativeContainer and CssBaselineContainer components as defined in AppContainer.ts
+//   CSS changes in those components can have implications for the functionality in this hook


This thing becomes so complex 🥲

I've decided to go with a way to let CSS carry the load here.

Have deleted this hook and created a full page container component

widget/packages/widget/src/components/FullPageContainer.tsx

Lines 9 to 14 in ddbb841

export const FullPageContainer = styled((props: PageContainerProps) => (

<PageContainer

{...props}

className={`${props.className} full-page-container`}

/>

))``;

The css for this lives in the AppContainer

widget/packages/widget/src/components/AppContainer.tsx

Lines 93 to 97 in ddbb841

[`&:has(.full-page-container)`]: {

height: theme.container?.maxHeight

? theme.container?.maxHeight

: theme.container?.height || defaultMaxHeight,

},

And just means on the pages that we swap out the PageContainer for the FullPageContainer

widget/packages/widget/src/pages/SelectChainPage/SelectChainPage.tsx

Line 69 in ddbb841

<FullPageContainer disableGutters>

This method was already being used for the SelectTokenPage by setting an id on the TokenList, I've just repurposed it here.

In relation to the layout/height code I think there are still somethings that we can do to simplify the code further in future.

Consolidate the logic and styles in the AppContainer.tsx files AppExpandedContainer, RelativeContainer and CssBaselineContainerProps. It does get confusing where the height should be set to get the desired result for layouts

useTokenListHeight hook could potentially be replaced with a css implementation - I've managed to create a list in a side project that resizes in the way that the token list in our app does using just css. Though I have had a failed attempt at getting this working already I do think that at some point its worth another attempt.

chybisov · 2024-09-25T09:08:13Z

packages/widget/src/components/Search/SearchInput.style.tsx

+
+interface SearchStickyContainerProps {
+  headerHeight: number;
+}
+
+export const searchContainerHeight = 64;
+
+// When the widget is in Full Height mode the


Something is missing at the end of the comment?

Have reworded this

chybisov · 2024-09-25T09:09:14Z

packages/widget/src/components/Search/SearchInput.style.tsx

+}));
+
+// When in full height mode the list needs make an addition to the paddingTop of the SearchStickyContainer height
+// this compensations for SearchStickyContainer being fixed position and compensates for it no long being in the document flow


It is not clear to me, especially the second part 😅

Have reworded this, hopefully it makes more sense

chybisov · 2024-09-25T09:12:48Z

packages/widget/src/hooks/useHeaderHeight.ts

+        resizeObserver.disconnect();
+      }
+    };
+  }, [elementId, headerHeight, headerElement, setHeaderHeight]);


One improvement I think that would still be good here is to move the useLayoutEffect to the Header component itself and have that set a value on the Header Store - the useHeaderHeight hook would then become an access point for the height value in the store and using zustand shallow checks would actually be able to limit potential renders in the Widget where the height value it used

Should we do it now? 👀

chybisov · 2024-09-25T09:14:42Z

packages/widget/src/stores/settings/useSettingsStore.ts

+          const allKeysInCollectionEnabled = toolKeys.every(
+            (toolKey) => state[`_enabled${toolType}`][toolKey],
+          );
+
+          // then toggle those keys to false
+          const updatedTools = toolKeys.reduce(
+            (accum, toolKey) => {
+              accum[toolKey] = !allKeysInCollectionEnabled;
+              return accum;
+            },
+            {
+              ...state[`_enabled${toolType}`],
+            },
+          );
+
+          const enableKeys: string[] = [];
+          const disabledKeys: string[] = [];
+
+          Object.entries(updatedTools).forEach(([key, value]) => {
+            if (value) {
+              enableKeys.push(key);
+            } else {
+              disabledKeys.push(key);
+            }
+          });
+
          return {
-            [`_enabled${toolType}`]: enabledTools,
-            [`enabled${toolType}`]: enableAll ? Object.keys(enabledTools) : [],
-            [`disabled${toolType}`]: enableAll ? [] : Object.keys(enabledTools),
+            [`_enabled${toolType}`]: updatedTools,
+            [`enabled${toolType}`]: enableKeys,
+            [`disabled${toolType}`]: disabledKeys,


Due to these changes here please double-check just in case that we send the correct filtering options to the API.

WIll check this in the morning

Have check that the bridge and exchanges are passed correctly to the options on request - it looks like its working

github-actions · 2024-09-25T19:39:02Z

Hey! This is your new endopint: https://f1b89902.widget-addsearchi.pages.dev

DNR500 · 2024-09-25T19:57:12Z

I've also added some fixes to make the WIdgetSkeleton look better in relation to the different layout modes

github-actions · 2024-09-25T19:59:31Z

Hey! This is your new endopint: https://cc9ce090.widget-addsearchi.pages.dev

DNR500 · 2024-09-26T10:37:56Z

packages/widget/src/components/TokenList/TokenList.tsx

-    <Box
-      ref={parentRef}
-      style={{ height, overflow: 'auto' }}
-      id={createElementId(ElementId.TokenList, elementId)}


This isn't needed any more as the SelectTokenPage used the FullPageContainer

DNR500 · 2024-09-26T10:39:37Z

packages/widget/src/components/Skeleton/WidgetSkeleton.style.tsx

+  paddingRight: theme.spacing(3),
+}));
+
+export const SkeletonHeaderContainer = styled(Box)(({ theme }) => {


Some widget skeleton changes were needed so that the Skeleton doesn't apear broken in the various layout modes

DNR500 added 8 commits September 23, 2024 17:21

feat: initial notes for implementation

5558c06

feat: keep chain search full page and add sticky input

8c915e5

feat: add in Not Found message

e6960a0

refactor: use search input with token list

71ca4ba

feat: add the search input ot the bridges and exchanges pages

58a6cf7

refactor: use mui's debounce

a7fcf66

fix: not found message position when sticky input in full height layout

e9b0a00

feat: reset the scroll to the top of the page when search updates

8855a0c

DNR500 added the WIP Work in progress label Sep 24, 2024

DNR500 added 2 commits September 24, 2024 17:38

feat: get check all box to work

ad6c360

fix: rendering in relation to the header position

27c0d24

DNR500 commented Sep 24, 2024

View reviewed changes

Merge branch 'main' into add-search-input-to-chain-bridges-exchanges-…

5d7c485

…pages

DNR500 marked this pull request as ready for review September 24, 2024 18:18

DNR500 added testing and removed WIP Work in progress labels Sep 24, 2024

chybisov reviewed Sep 25, 2024

View reviewed changes

DNR500 added 4 commits September 25, 2024 18:29

refactor: header height functionality

2acd7ae

docs: update comment and rename container component

4fb4cd5

refactor: place hook with full page container

1949b50

fix: widget skeleton in relation to layouts

ddbb841

docs: update comment

a517c72

DNR500 commented Sep 26, 2024

View reviewed changes

chybisov merged commit 5504d95 into main Sep 30, 2024
1 check passed

chybisov deleted the add-search-input-to-chain-bridges-exchanges-pages branch September 30, 2024 12:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat Add search input to chain, bridges and exchanges pages #305

Feat Add search input to chain, bridges and exchanges pages #305

DNR500 commented Sep 24, 2024 •

edited by jira bot

Loading

DNR500 Sep 24, 2024

chybisov Sep 25, 2024

chybisov Sep 25, 2024

DNR500 Sep 25, 2024

github-actions bot commented Sep 24, 2024

chybisov Sep 25, 2024

chybisov Sep 25, 2024

DNR500 Sep 25, 2024

DNR500 Sep 25, 2024 •

edited

Loading

chybisov Sep 25, 2024

DNR500 Sep 25, 2024 •

edited

Loading

chybisov Sep 25, 2024

DNR500 Sep 25, 2024

chybisov Sep 25, 2024

chybisov Sep 25, 2024

DNR500 Sep 25, 2024

DNR500 Sep 26, 2024

github-actions bot commented Sep 25, 2024

DNR500 commented Sep 25, 2024

github-actions bot commented Sep 25, 2024

DNR500 Sep 26, 2024

DNR500 Sep 26, 2024

	export const FullPageContainer = styled((props: PageContainerProps) => (
	<PageContainer
	{...props}
	className={`${props.className} full-page-container`}
	/>
	))``;

	[`&:has(.full-page-container)`]: {
	height: theme.container?.maxHeight
	? theme.container?.maxHeight
	: theme.container?.height \|\| defaultMaxHeight,
	},

Feat Add search input to chain, bridges and exchanges pages #305

Feat Add search input to chain, bridges and exchanges pages #305

Conversation

DNR500 commented Sep 24, 2024 • edited by jira bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DNR500 Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DNR500 Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 25, 2024

DNR500 commented Sep 25, 2024

github-actions bot commented Sep 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DNR500 commented Sep 24, 2024 •

edited by jira bot

Loading

DNR500 Sep 25, 2024 •

edited

Loading

DNR500 Sep 25, 2024 •

edited

Loading