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

Fix #1596 (Remove Loading Animation) #1626

Merged
merged 5 commits into from
Oct 24, 2023

Conversation

Sranjan0208
Copy link
Contributor

@Sranjan0208 Sranjan0208 commented Oct 4, 2023

Added validation to the router to prevent showing loading animation

Changes proposed ✍️

This pull request addresses the issue of displaying the loading animation even when the user navigates to the same route. To achieve this, I've added a condition to check if the destination route (to.name) is different from the current route (from.name). If they are the same, the loading animation won't be triggered.

What

🤖 Generated by Copilot at ca9b2d6

Fixed a bug where the progress bar would show up when navigating to the same route in the frontend. Updated the logic in frontend/src/router/index.js to only start the progress bar when the route name changes.

🤖 Generated by Copilot at ca9b2d6

routeName changes
progress bar shows or hides
autumn leaves falling

Why

How

🤖 Generated by Copilot at ca9b2d6

  • Prevent progress bar from showing up when navigating to the same route (link)

Checklist ✅

  • Label appropriately with Feature, Improvement, or Bug.
  • Add screehshots to the PR description for relevant FE changes
  • New backend functionality has been unit-tested.
  • API documentation has been updated (if necessary) (see docs on API documentation).
  • Quality standards are met.

Added validation to the router to prevent showing loading animation
@Sranjan0208 Sranjan0208 changed the title Fix #1596 Fix #1596 (Remove Loading Animation) Oct 4, 2023
Copy link
Contributor

@joanagmaia joanagmaia left a comment

Choose a reason for hiding this comment

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

LGTM 🚀
Will deploy it on monday 😄

@joanagmaia
Copy link
Contributor

Just missing to fix the conflicts

@joanagmaia
Copy link
Contributor

@Sranjan0208 could you just solve the conflicts so that I can merge 😄 ?

@joanagmaia
Copy link
Contributor

Sorry again @Sranjan0208, was gonna merge this one now but Lint is failing. Can you run the npm run lint to check it out?

@joanagmaia
Copy link
Contributor

Hey @Sranjan0208, in the root of frontend you can run npm run lint to check the files that need fixing. The lint frontend check is failing 😄

@Sranjan0208
Copy link
Contributor Author

I ran "npm run lint", and made the necessary changes required. Can you check if it's working fine now. Thanks

@joanagmaia joanagmaia merged commit 5d7e11e into CrowdDotDev:main Oct 24, 2023
4 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.

2 participants