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

calcite-list - initial empty list with loading=true is not visually tall enough to show loading animation #7147

Closed
jgrayson-apl opened this issue Jun 6, 2023 · 13 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. design Issues that need design consultation prior to development. estimate - 3 A day or two of work, likely requires updates to tests. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive p - low Issue is non core or affecting less that 10% of people using the library ready for dev Issues ready for development implementation.

Comments

@jgrayson-apl
Copy link

Actual Behavior

When an empty calcite-list with loading=true is initially displayed it's not visually tall enough to show loading animation.

Expected Behavior

I would expect the calcite-list to be tall enough to display the loading animation even if the list is empty

Reproduction Sample

https://codepen.io/john-grayson/pen/gOQOmZE

Reproduction Steps

  1. Load the above codepen
  2. Watch how the list is initially not tall enough to display the loading animation

Reproduction Version

1.4.2

Relevant Info

No response

Regression?

No response

Priority impact

p4 - not time sensitive

Impact

No response

Esri team

N/A

@jgrayson-apl jgrayson-apl added 0 - new New issues that need assignment. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. needs triage Planning workflow - pending design/dev review. labels Jun 6, 2023
@github-actions github-actions bot added the impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive label Jun 6, 2023
@geospatialem
Copy link
Member

Also appears a bit cut off when only one list-item is present: https://codepen.io/geospatialem/pen/MWzWmzQ.

@geospatialem geospatialem added the design Issues that need design consultation prior to development. label Jun 13, 2023
@geospatialem
Copy link
Member

@SkyeSeitz @ashetland What would you folks recommend when the list is empty to display the loading spinner? We'd like to tackle this one for the June priorities this month.

@geospatialem geospatialem added estimate - 3 A day or two of work, likely requires updates to tests. p - low Issue is non core or affecting less that 10% of people using the library and removed needs triage Planning workflow - pending design/dev review. labels Jun 13, 2023
@jgrayson-apl
Copy link
Author

FYI: I updated my codepen with the workaround I'm using in my apps. I basically unset/reset the 'position' style property of the internal container.

@ashetland
Copy link
Contributor

Could List accommodate the Loader with some minimal padding? In this example, the empty List would be 80px tall while loading.

CleanShot 2023-06-13 at 16 15 50@2x

@driskull
Copy link
Member

Yes, that is super doable.

@driskull
Copy link
Member

I think it would be better if the calcite-scrim was just more responsive to adjust the loading spinner size relative to available height. That way, the list would't get taller just for a loading spinner if only 1 item exists.

@driskull
Copy link
Member

When the loader component inside the scrim is is set to scale="s" it looks good.

image

@ashetland
Copy link
Contributor

Love that idea. Since Loader has scales I wasn't sure if we were trying to be consistent with using a m Loader with a m List.

Would we still need to define a minimum height if there are no Items in the List while it's loading? We could go the height of a single List Item, like in your example.

@ashetland
Copy link
Contributor

ashetland commented Jun 14, 2023

And assuming List gets small and large scales added to it eventually, we'd probably need to ensure enough height at small scale for the small scale Loader inside Scrim.

@ashetland ashetland added ready for dev Issues ready for development implementation. and removed design Issues that need design consultation prior to development. labels Jun 14, 2023
@driskull
Copy link
Member

@alisonailea @ashetland if we define heights in which the loading spinner switches from "m" to "s" or "l" do we need these heights defined in the design tokens somewhere?

If not, could you give me height number breakpoints for when these size switches should occur?

@ashetland
Copy link
Contributor

ashetland commented Jun 15, 2023

Okay we think we have a good starting point for this. These breakpoints can be adjusted in the future if/when we get feedback on it.

Breakpoints would be 72px and 480px:

CleanShot 2023-06-15 at 16 21 12@2x

Demo video:

CleanShot.2023-06-15.at.16.32.29.mp4

@ashetland ashetland removed their assignment Jun 15, 2023
driskull added a commit that referenced this issue Jun 17, 2023
**Related Issue:** #7147

## Summary

- Updates the scrim to set the scale of the internal loading spinner
based on the scrim size.
- Scrim is now responsive with a resize observer.
- Breakpoints added based on design. These could maybe go into some kind
of component token in the future.
- Breakpoint is based on the minimum value of width or height.
- Adds test
@driskull driskull added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 0 - new New issues that need assignment. labels Jun 17, 2023
@github-actions
Copy link
Contributor

Installed and assigned for verification.

@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Jun 26, 2023
@geospatialem
Copy link
Member

Verified with https://codepen.io/geospatialem/pen/MWzWmzQ in 1.5.0-next.4.

@brittneytewks brittneytewks added the design Issues that need design consultation prior to development. label Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. design Issues that need design consultation prior to development. estimate - 3 A day or two of work, likely requires updates to tests. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive p - low Issue is non core or affecting less that 10% of people using the library ready for dev Issues ready for development implementation.
Projects
None yet
Development

No branches or pull requests

5 participants