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

feat(organizations): add theming/story for Mantine Loader and LoadingOverlay TASK-1464 #5425

Merged
merged 2 commits into from
Jan 21, 2025

Conversation

pauloamorimbr
Copy link
Contributor

@pauloamorimbr pauloamorimbr commented Jan 17, 2025

🗒️ Checklist

  1. run linter locally
  2. update all related docs (API, README, inline, etc.), if any
  3. draft PR with a title <type>(<scope>)<!>: <title> TASK-1234
  4. tag PR: at least frontend or backend unless it's global
  5. fill in the template below and delete template comments
  6. review thyself: read the diff and repro the preview as written
  7. open PR & confirm that CI passes
  8. request reviewers, if needed
  9. delete this section before merging

📣 Summary

Theming for Mantine's Loader and LoadingOverlay was added to match Kobo's current LoadingSpinner for general purpose.

📖 Description

Both 'regular' and 'big' variant were added with its respective type names to be used with Mantine's Loader and LoadingOverlay components.
To simplify the implementation I decided to simply use the existing LoadingSpinner component instead of extracting it's contents.
Since Mantine's Loader doesn't have the message parameter, right now I'm fixing it on false so it won't appear. We may need to implement a Wrapper in the future to add a message, depending on our future needs.

👀 Preview steps

  • Use storybook to check the visual of the component and compare with the existing loader in use in the app.

Copy link
Contributor

@jamesrkiger jamesrkiger left a comment

Choose a reason for hiding this comment

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

@pauloamorimbr pauloamorimbr force-pushed the pamorim/task-1464-mantine-theme-for-loader branch from 2b9fbd0 to 95b7561 Compare January 21, 2025 20:25
@pauloamorimbr
Copy link
Contributor Author

Should we deprecate this? https://github.com/kobotoolbox/kpi/blob/pamorim/task-1464-mantine-theme-for-loader/jsapp/js/components/common/loadingSpinner.stories.tsx

Other than that, looks good to me.

I don't think so... I'm just using the default mantine component, which does not use the message parameter. Instead of deprecating the current one with a message we should probably make a wrap so Mantine's one would properly replace it.

@pauloamorimbr pauloamorimbr force-pushed the pamorim/task-1464-mantine-theme-for-loader branch from 95b7561 to b9b0953 Compare January 21, 2025 20:39
@pauloamorimbr pauloamorimbr merged commit a72f2ef into main Jan 21, 2025
7 checks passed
@pauloamorimbr pauloamorimbr deleted the pamorim/task-1464-mantine-theme-for-loader branch January 21, 2025 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants