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

bugfix-> spinner align to center #1908

Merged
merged 5 commits into from
Sep 10, 2024
Merged

bugfix-> spinner align to center #1908

merged 5 commits into from
Sep 10, 2024

Conversation

Yagnik56
Copy link
Collaborator

@Yagnik56 Yagnik56 commented Sep 7, 2024

This PR align feature flags the root page spinner in the center.
Also changed Search placeholder string as per Figma.

@Yagnik56 Yagnik56 self-assigned this Sep 7, 2024
@Yagnik56 Yagnik56 linked an issue Sep 9, 2024 that may be closed by this pull request
@zackcl
Copy link
Collaborator

zackcl commented Sep 9, 2024

@Yagnik56 @danoswaltCL I'm wondering when we display the loading progress bar above the table and when we display the spinner. It seems like the spinner is visible only the first time the page is refreshed. Could we make it consistently display the loading progress bar for loading the table content instead of using the spinner unless it's really necessary? If you update this.featureFlagService.fetchFeatureFlags(); to this.featureFlagService.fetchFeatureFlags(true); and click on "Feature Flags" from the sidenav, you will always see the loading progress bar whenever the feature flags data is refetched. I wonder if it's possible to make it always work like this, even after the page is refreshed.

@zackcl
Copy link
Collaborator

zackcl commented Sep 9, 2024

If that's not possible, I think it might be better to show the spinner at the center of the page without displaying the empty section card, similar to how the Experiments page works. I believe this approach is better and safer unless we want to allow users to interact with the page before loading is complete.

Screenshot 2024-09-09 at 9 46 27 PM Screenshot 2024-09-09 at 9 46 41 PM

@zackcl
Copy link
Collaborator

zackcl commented Sep 9, 2024

The PR is fine, but I'm just throwing out some questions and ideas. We might discuss this in the meeting.

@zackcl
Copy link
Collaborator

zackcl commented Sep 9, 2024

While we decided to keep the spinner as it is for now, I would like to suggest reducing the size of the spinner and adding padding to the spinner container for consistency and a more natural look.

@Yagnik56 Could you apply these two changes?

  • Update the mat-spinner tag from <mat-spinner></mat-spinner> to <mat-spinner diameter="60"></mat-spinner> in feature-flag-root-section-card.component.html
  • Add padding: 64px; inside the .mat-spinner-container {} in feature-flag-root-section-card.component.scss

This will make the spinner to be shown like this while loading:

Screenshot 2024-09-09 at 11 33 50 PM

danoswaltCL
danoswaltCL previously approved these changes Sep 9, 2024
@zackcl zackcl dismissed stale reviews from danoswaltCL and VivekFitkariwala via 590fe2b September 10, 2024 08:18
@zackcl
Copy link
Collaborator

zackcl commented Sep 10, 2024

@Yagnik56 I just made a commit to update the style since it's a minor change.

@zackcl zackcl merged commit 9b1c333 into dev Sep 10, 2024
14 checks passed
@zackcl zackcl deleted the bugfix/align-spinner-in-center branch September 10, 2024 08:27
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.

Feature Flag Root Section Card
4 participants