Floating Data Catalog Filters Sidebar Follow-up and starting some cleaning and modularizing #930

sandrahoang686 · 2024-04-30T22:57:51Z

Related Ticket:

Closes #926

Description of Changes

Make filters sidebar height 100% when filters sidebar height (when all expanded) is greater than or equaled to the height of all the cards
Calculate filters sidebar height with position:sticky when filters sidebar height (when all expanded) is less than height all of cards
Display all filters opened now
Move prepareDatasets to own logic file - cleaning
Move layout components from Data Catalog component into the wrapper - starting to modularize

Notes & Questions About Changes

DEMO Here:
https://www.loom.com/share/5f88edba7ca4422d9f2636464cebf7b6?sid=62ae3536-4174-4de0-adfe-a19b2e4a5893

Validation / Testing

Make sure when the height of the filters sidebar (when expanded) is greater than the height of all the cards, the sidebar doesn't behave sticky
Make sure when the height of the filters sidebar (when expanded) is less than the height of all the cards, the sidebar behaves sticky

netlify · 2024-04-30T22:58:10Z

✅ Deploy Preview for veda-ui ready!

Name	Link
🔨 Latest commit	`8f88d13`
🔍 Latest deploy log	https://app.netlify.com/sites/veda-ui/deploys/6638eeff57e9d50008853bab
😎 Deploy Preview	https://deploy-preview-930--veda-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

sandrahoang686 · 2024-05-01T18:15:46Z

Demo of how this looks in the GHG instance will be following soon...

j08lue · 2024-05-01T18:50:27Z

Hmm, does not float for me, I think (latest Brave)?

sandrahoang686 · 2024-05-01T18:58:07Z

Demo here:
https://www.loom.com/share/5f88edba7ca4422d9f2636464cebf7b6?sid=62ae3536-4174-4de0-adfe-a19b2e4a5893
@j08lue does this ^ help answer your comment. I'm guessing you closed all the filters and was hoping it would still be sticky? I calculate the height of all the filters when they are open and if all the filters height is greater than height of all cards, position sticky doesn't apply. Let me know if this behavior is fine though or if you prefer differently?

j08lue · 2024-05-01T19:55:01Z

Very nice and helpful demo, thanks!

I'm guessing you closed all the filters and was hoping it would still be sticky?

Exactly

I calculate the height of all the filters when they are open and if all the filters height is greater than height of all cards, position sticky doesn't apply.

I imagine this is quite tricky, because you may also need to scroll the side bar in some cases and it does not have its own scroll bar? 🤯

Let me test a bit more on a GHG Center preview (I can set that up shortly) and also see what @faustoperez says, before you make any more changes.

j08lue · 2024-05-01T19:59:35Z

Created a GHG Center preview here:

[DO NOT MERGE] Preview of floating Data Catalog sidebar US-GHG-Center/veda-config-ghg#359

faustoperez

Ah I see it's more complicated than I thought in the first place, as the height of the element with the accordions and the parent need to be calculated for it to work 🤯 .

To move things forward, I am approving this PR, but let's revisit the behavior after the v2 release.

Ideally, the sidebar would stick at the bottom of the main content when scrolling, it wouldn't scroll up.

Here's a plugin I found in case it helps: https://github.com/blixhavn/sticky-sidebar-v2?tab=readme-ov-file

And a demo: https://blixhavn.github.io/sticky-sidebar-v2/examples/scrollable-element.html

j08lue · 2024-05-03T09:28:11Z

Ideally, the sidebar would stick at the bottom of the main content when scrolling, it wouldn't scroll up.

+1

hanbyul-here · 2024-05-03T19:57:57Z

I pushed some changes that include

"Sticking" to the bottom when content is scrolling - (@faustoperez can you check if this is the behavior you expect to see? )
Change sticky behavior depending on the height of the sidebar
Change the sticking point depending on the page header status

position: sticky does a lot by default (ex. do not stick when a sticky element is longer than the contents) I think what this PR was missing was to give an actual height value to the sticky element. I got rid of the logic of comparing the heights of 'area' and 'sidebar' since they are not needed anymore. I did not clean up further than data-catalog/filters-control, data-catalog/index , so a further clean-up might be needed.

hanbyul-here · 2024-05-03T20:07:30Z

app/scripts/components/common/form/checkable-filter/index.tsx

  const [count, setCount] = useState<number>(0);
  const [selected, setSelected] = useState<OptionItem[]>([]);
+  const targetRef = React.useRef<HTMLDivElement>(null);


Is there any need to introduce the pattern of using React.useRef? I am not sure we can set it up as a lint rule, but all other parts of the code explicitly import {useRef}

not a pattern, can just explicitly import or use like this

hanbyul-here · 2024-05-03T20:11:34Z

app/scripts/components/data-catalog/index.tsx

    setAllSelectedFilters([]);
  }, [setAllSelectedFilters]);

-  React.useEffect(() => {
+  useEffect(() => {
    if (clearedTagItem && (allSelectedFilters.length == prevSelectedFilters.length-1)) {
      onAction(Actions.TAXONOMY_MULTISELECT, { key: clearedTagItem.taxonomy, value: clearedTagItem.id}); 
      setClearedTagItem(undefined);
    }
  }, [allSelectedFilters, clearedTagItem]);


I noticed warnings about useEffect has missing dependencies throughout this component. Do you mind putting comments on why those dependencies are excluded? It would help me navigate the code better.

Sure. We have missing dependencies sprinkled throughout the existing codebase already. We can start making notes going forward why some arn't included and as a team create standardization around this in-code documentation

j08lue · 2024-05-06T18:09:19Z

Awesome, the scrolling experience is great now - the filters are visible until the very end of the dataset list and the behavior takes into account the actual height of the side bar. This way, users will always be able to see the filters.

One deviation from the design is that the pills that indicate the active filters and give a shortcut to removing them are added in the right column, above the dataset list:

Instead of between search bar and filters:

That means that the shortcut to removing individual filters disappears from view quite quickly as the user scrolls down the filtered list. Was this a change you discussed, @faustoperez and @sandrahoang686?

We could keep that refinement to yet another PR, though, and ship this increment already. 👌

sandrahoang686 · 2024-05-06T18:46:49Z

@j08lue yes that was a change Fausto and I discussed this during our checkin and in the original PR to have the tags above the dataset list instead of above the filters. Decision change documented here

j08lue

As noted in #930 (comment), the implementation works very nicely!

The small difference from the designs in terms of placement of the filter removal pills is ok for now for me.

### Features * Floating Data Catalog Filters Sidebar Follow-up and starting some cleaning and modularizing in #930 * Make geometries non-movable in the Exploration feature in #936

Start making filters sidebar sticky and move prepareDatasets logic

31b9740

Calculate the style height

17c0906

sandrahoang686 requested review from hanbyul-here and j08lue May 1, 2024 18:12

sandrahoang686 marked this pull request as ready for review May 1, 2024 18:12

sandrahoang686 changed the title ~~Sticky floating data-catalog filters sidebar~~ Floating Data Catalog Filters Sidebar Follow-up and starting some cleaning and modularizing May 1, 2024

Update calc total

622bd55

j08lue mentioned this pull request May 1, 2024

[DO NOT MERGE] Preview of floating Data Catalog sidebar US-GHG-Center/veda-config-ghg#359

Closed

sandrahoang686 requested a review from faustoperez May 2, 2024 13:52

faustoperez approved these changes May 3, 2024

View reviewed changes

sandrahoang686 requested a review from dzole0311 May 3, 2024 13:21

Fix height style syntax

7f51c39

hanbyul-here force-pushed the update/data-catalog-make-sidebar-floating branch 2 times, most recently from ae04d76 to 42cbe65 Compare May 3, 2024 19:38

Improve sticky behavior, Make syntax coherent

1e111b2

hanbyul-here force-pushed the update/data-catalog-make-sidebar-floating branch from 42cbe65 to 1e111b2 Compare May 3, 2024 19:43

hanbyul-here reviewed May 3, 2024

View reviewed changes

More clean up

8f88d13

sandrahoang686 merged commit db802a1 into main May 6, 2024
8 checks passed

sandrahoang686 deleted the update/data-catalog-make-sidebar-floating branch May 6, 2024 18:58

j08lue approved these changes May 6, 2024

View reviewed changes

hanbyul-here mentioned this pull request May 6, 2024

Bump up the version to 4.4 #938

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Floating Data Catalog Filters Sidebar Follow-up and starting some cleaning and modularizing #930

Floating Data Catalog Filters Sidebar Follow-up and starting some cleaning and modularizing #930

sandrahoang686 commented Apr 30, 2024 •

edited

Loading

netlify bot commented Apr 30, 2024 •

edited

Loading

sandrahoang686 commented May 1, 2024 •

edited

Loading

j08lue commented May 1, 2024

sandrahoang686 commented May 1, 2024 •

edited

Loading

j08lue commented May 1, 2024

j08lue commented May 1, 2024

faustoperez left a comment

j08lue commented May 3, 2024

hanbyul-here commented May 3, 2024 •

edited

Loading

hanbyul-here May 3, 2024

sandrahoang686 May 6, 2024

hanbyul-here May 3, 2024

sandrahoang686 May 6, 2024 •

edited

Loading

j08lue commented May 6, 2024 •

edited

Loading

sandrahoang686 commented May 6, 2024

j08lue left a comment

Floating Data Catalog Filters Sidebar Follow-up and starting some cleaning and modularizing #930

Floating Data Catalog Filters Sidebar Follow-up and starting some cleaning and modularizing #930

Conversation

sandrahoang686 commented Apr 30, 2024 • edited Loading

Description of Changes

Notes & Questions About Changes

Validation / Testing

netlify bot commented Apr 30, 2024 • edited Loading

✅ Deploy Preview for veda-ui ready!

sandrahoang686 commented May 1, 2024 • edited Loading

j08lue commented May 1, 2024

sandrahoang686 commented May 1, 2024 • edited Loading

j08lue commented May 1, 2024

j08lue commented May 1, 2024

faustoperez left a comment

Choose a reason for hiding this comment

j08lue commented May 3, 2024

hanbyul-here commented May 3, 2024 • edited Loading

hanbyul-here May 3, 2024

Choose a reason for hiding this comment

sandrahoang686 May 6, 2024

Choose a reason for hiding this comment

hanbyul-here May 3, 2024

Choose a reason for hiding this comment

sandrahoang686 May 6, 2024 • edited Loading

Choose a reason for hiding this comment

j08lue commented May 6, 2024 • edited Loading

sandrahoang686 commented May 6, 2024

j08lue left a comment

Choose a reason for hiding this comment

sandrahoang686 commented Apr 30, 2024 •

edited

Loading

netlify bot commented Apr 30, 2024 •

edited

Loading

sandrahoang686 commented May 1, 2024 •

edited

Loading

sandrahoang686 commented May 1, 2024 •

edited

Loading

hanbyul-here commented May 3, 2024 •

edited

Loading

sandrahoang686 May 6, 2024 •

edited

Loading

j08lue commented May 6, 2024 •

edited

Loading