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 loading wheel, shoutouts #507

Merged
merged 7 commits into from
Oct 2, 2023
Merged

Conversation

kevinmram
Copy link
Contributor

@kevinmram kevinmram commented Sep 28, 2023

Ticket: "Shoutouts - Fix How Shoutouts Are Loaded In on Admin View"
Notion: https://www.notion.so/cornelldti/Idol-FA23-08f1aac52a364b1dad8cde77ebc1c027?p=c402189001014a1798a22db9286d4566&pm=s

implemented Loader from Semantic UI into "Shoutouts" - Previously, users were presented with empty space while waiting for shoutouts to load, leading to confusion regarding the application's state. The incorporation of a loading indicator serves to improve the user experience by providing real-time feedback and a more intuitive interaction.

shoutoutLoader

@kevinmram kevinmram requested a review from a team as a code owner September 28, 2023 02:01
@dti-github-bot
Copy link
Member

dti-github-bot commented Sep 28, 2023

[diff-counting] Significant lines: 46.

@andrew032011
Copy link
Collaborator

andrew032011 commented Sep 28, 2023

Format check is failing. Run the formatter with yarn run format.

@andrew032011
Copy link
Collaborator

andrew032011 commented Sep 28, 2023

Please include the notion ticket associated with the PR. That makes it easier for the reviewer (and future readers of the repo) to navigate back to and see the scope of the task you're working on + context.

@andrew032011
Copy link
Collaborator

I like these changes! The Notion ticket specifies to fix this loading issue for the Admin side, not the member side (but these changes are much appreciated and we should still keep them). Can you also add the loading wheel to the admin view?

</Button.Group>
</Form.Group>
</Form>
<div className={styles.shoutoutsListContainer}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you instead wrap <div className={styles.shoutoutsListContainer}></div> with the ternary with Loader? That way we can see a least some part of the page before everything loads in.

Copy link
Contributor

@patriciaahuang patriciaahuang left a comment

Choose a reason for hiding this comment

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

Great job, Kevin! I can see the top of the page while the shoutouts are loading with the loader.

Copy link
Collaborator

@andrew032011 andrew032011 left a comment

Choose a reason for hiding this comment

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

Just run yarn run format and you're golden!

@kevinmram kevinmram merged commit d759996 into main Oct 2, 2023
13 checks passed
@kevinmram kevinmram deleted the u/kevin/add-shoutout-loading-wheel branch October 2, 2023 20:44
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.

4 participants