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

Add a web widget for the activity indicator (shoelace spinner) #2050

Merged
merged 6 commits into from Jul 26, 2023

Conversation

ahter
Copy link
Contributor

@ahter ahter commented Jul 23, 2023

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@ahter ahter force-pushed the add-progress-ring branch 3 times, most recently from 5244c21 to 98c2ba6 Compare July 23, 2023 14:17
This is the web implementation of the spinner component.

The details can be found in:
https://shoelace.style/components/spinner
Prior to this change, ActivityIndicator wasn't part
of the factory for the web example apps.

This change imports and adds it so that it can be
tested out.
@ahter ahter changed the title [Very-WIP] Add a web widget for the activity indicator Add a web widget for the activity indicator (Shoelace spinner) Jul 23, 2023
@ahter ahter changed the title Add a web widget for the activity indicator (Shoelace spinner) Add a web widget for the activity indicator (shoelace spinner) Jul 23, 2023
This was necessary to override the behavior of adding
flex element to the style. The spinner (activity indicator)
needs to have this set to false.
if (
(self.width == NONE and self.direction == ROW)
or (self.height == NONE and self.direction == COLUMN)
) and getattr(self, "_use_default_width", True):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For posterity: this approach was developed pairing with @ahter.

The underlying problem is that if the ActivityIndicator has no Toga style, it falls back to rendering as flex: 0 0 0px, which evaluates as 0 width in CSS. If you apply a fixed width of 20px, the animation for the spin and the gray background become misaligned. We need to work out how to:

  1. Apply a style override to the ActivityIndicator so that it doesn't collapse to 0 size, or
  2. Work out how to inject a widget-specific modification to Pack's __css__ implementation to disable the default

It feels to me like both are needed. (1) is needed because if a user applies a "width=100" pack style, we don't want the widget to render weird; (2) is needed to make sure that the default style results in a visible widget.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good; however, we need to find a better approach for the styling override issue.

FYI: The Linux CI failure will be fixed by #2052. The Android CI failure will be fixed by beeware/briefcase#1380.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found an alternate workaround; flex 0 0 0 (implying flex-basis: 0) means that in the absence of a width setting, the box model assigns a size of 0 to the widget. flex 0 0 auto means a flex-basis of auto, meaning the widget's intrinsic width will be used.

@freakboy3742 freakboy3742 merged commit f64c992 into beeware:main Jul 26, 2023
40 checks passed
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.

None yet

2 participants