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

WidgetList component #794

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

WidgetList component #794

wants to merge 3 commits into from

Conversation

Rogermax
Copy link
Contributor

@Rogermax Rogermax commented Nov 5, 2024

🚪 Why?

Make foundations for a better widget layout

🔑 What?

New component WidgetList

🏡 Context

Screen.Recording.2024-11-05.at.13.33.15.mov

@Rogermax Rogermax changed the title wip WidgetList component Nov 5, 2024
Copy link

github-actions bot commented Nov 5, 2024

🔍 Visual review for your branch is published 🔍

Here are the links to:

@nlopin
Copy link
Contributor

nlopin commented Nov 5, 2024

I have doubts about the approach, but because there is a PR and a lot of work already done, it is hard to share it. What do you think if next time we discuss the approach before writing the code?

I see the value in providing the default shell for widget, especially that many of them use this concept of "+n more". At the same time, the current approach relies on setting a maximum height as a way to control the content which does not give us enough sense of the amount of data user will see. I think the height of a widget is depends on the content, meaning the content dictates the height, not the other way around.

In addition, the manual calculation of the height is a performance bottleneck, since offsetHeight triggers reflow (more about this on google dev blog)

We already have a bunch of list component and WidgetList could be a wrapper of them but giving an obvious API. The developer can provide the amount of items they want to show and optionally the amount of items to show in the "+n items" text together with the url where to redirect the user. If the number is not provided, we calculate it from the list of items provided.

@Rogermax
Copy link
Contributor Author

Rogermax commented Nov 5, 2024

I have doubts about the approach, but because there is a PR and a lot of work already done, it is hard to share it. What do you think if next time we discuss the approach before writing the code?

I see the value in providing the default shell for widget, especially that many of them use this concept of "+n more". At the same time, the current approach relies on setting a maximum height as a way to control the content which does not give us enough sense of the amount of data user will see. I think the height of a widget is depends on the content, meaning the content dictates the height, not the other way around.

In addition, the manual calculation of the height is a performance bottleneck, since offsetHeight triggers reflow (more about this on google dev blog)

We already have a bunch of list component and WidgetList could be a wrapper of them but giving an obvious API. The developer can provide the amount of items they want to show and optionally the amount of items to show in the "+n items" text together with the url where to redirect the user. If the number is not provided, we calculate it from the list of items provided.

I was trying to make a better framework to avoid having to handle in every component the same issues with height, check this comment -> #753 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants