UI Scroll Issue Fix

[andrewsrajasekar]

What was the Issue?
There was a scroll Issue in OwnerOf Page where if the content height was too long, the scroll bar is hidden and pagination is below and we cannot scroll below to see the content

What Did I Do?
Fixed this issue by calculating the height dynamically and fixing it for the content

Screenshots for the issue:
The Bug:

The Fix:

Kindly take a look and let me know of any changes!
Thanks,
Andrews!

[codecov]

Bundle Report

Changes will increase total bundle size by 584 bytes (0.0%) ⬆️. This is within the configured threshold ✅

Detailed changes

Bundle name	Size	Change
datahub-react-web-esm	28.48MB	584 bytes (0.0%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: datahub-react-web-esm

Assets Changed:

Asset Name	Size Change	Total Size	Change (%)
assets/index-*.js	584 bytes	18.83MB	0.0%

Files in assets/index-*.js:

• ./src/app/entityV2/shared/components/styled/search/EntitySearchResults.tsx → Total Size: 4.35kB

• ./src/app/entityV2/shared/components/styled/search/EmbeddedListSearchResults.tsx → Total Size: 8.13kB

• ./src/app/searchV2/SearchablePage.tsx → Total Size: 3.99kB

[yoonhyejin]

Hey, thanks a lot for raising the detailed bug report & solid PR on this critical issue!

I was wondering if we could simplify the fix a bit — for example, adding a height in ResultContainer in EmbeddedListSearchResults.tsx like:

const ResultContainer = styled.div`
    ...
    height: calc(80vh - 60px);
    ...
`;

Could you check if this would cover the cases you mentioned? (pinging @jayacryl for second opinion here)

Also, looks like some lint is failing — you can try running:

./gradlew :datahub-web-react:yarnLintFix && ./gradlew :datahub-web-react:yarnLint

which should auto-fix most of them.

[jayacryl]

Hi Andrew! Thanks for sharing this.
We're trying to move away from javascript calculated heights, in favor of flex box.
@yoonhyejin even calc in css is kind of messy, because you need to calculate the height of the items above the ResultContainer.

What I recommend is doing something where the ResultsContainer ends up having a well defined height.
Ie.,

1. Prevent overflow for the parent SearchBody component.
2. Give ResultContainer height: 100% and overflow-y: auto

You may have to experiment with getting the right styles on the parent and child ResultContainer component to get the structure exactly how css needs to automatically scroll.

Generally Claude is really good at giving you hints of what might need to be changed to get this right - would recommend x-checking with claude!

[andrewsrajasekar]

Hello @jayacryl and @yoonhyejin,

Why the PR was closed?
Mistakenly Rebased my forked repo and hence this pr got closed and hence reopened.

Coming back to our fixes, what changed?
I have removed this dynamic height check and injecting into child component and have used flex and column arrangement to smooth things out.

Blocker encountered? FYI

1. UserAssets height was not restricted to the height of parent component and hence overflow: auto -> to restrict this to parent component height!
2. Adding height: 100%; to Routed Tabs -> made overflow-y(i.e) scroll appear at our ResultContainer Component(i.e) our user owner entity component!

@yoonhyejin I have ran ./gradlew :datahub-web-react:yarnLintFix && ./gradlew :datahub-web-react:yarnLint and it seems no changes have been made on my codebase!

Kindly take a look and let me know of any changes!
Thanks,
Andrews!

[jayacryl]

This looks good Andrew, much appreciated! I have tested locally and it seems good. One last request and we can merge:
Can you:

1. List out test cases both on this UI, and on other key routes that are affected by the broader change on 'route tabs'
2. Provide a quick screen recording in the PR running through these UI scenarios to validate there is no regression

Since there is a change on a broadly used file - re route tabs, we just want to be sensitive about regressions. Also do you know why the smoke tests might be failing? We'll need to confirm that the smoke failures are not related to this change.