Skip to content
This repository has been archived by the owner on Jun 10, 2024. It is now read-only.

Fixes(milestone): Multiple Milestone on spam clicking. #9713

Merged
merged 18 commits into from
Jan 12, 2024
Merged

Fixes(milestone): Multiple Milestone on spam clicking. #9713

merged 18 commits into from
Jan 12, 2024

Conversation

malay44
Copy link
Contributor

@malay44 malay44 commented Nov 1, 2023

Introduce isDisabled state to manage/milestone page for save button control.

Fixes Issue

Closes #9687

Changes proposed

  • Added a isDisabled state to disable the save button upon clicking it.

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

Note to reviewers

I am having errors while running the tests in gitpod, but I have tested my changes in a self-hosted version.

@malay44
Fixes(milestone): Introduce isDisabled state
5f24562 
In `manage/milestone` page for save button control.
@github-actions github-actions bot added the issue linked Pull Request has issue linked label Nov 1, 2023
sital002
sital002 reviewed Nov 1, 2023
View reviewed changes
Copy link
Member

@sital002 sital002 left a comment

Choose a reason for hiding this comment

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

Tested on:
OS: Ubuntu
Browser: Firefox
works well maybe we could add a disabled cursor on Button component

@eddiejaoude
Copy link
Member

Looks good 👍

I like Sital's suggestion 👍

@malay44
refactor disable save button on clicking
c224801 
conditionally adds a class after clicking save button one time
@sital002
Copy link
Member

sital002 commented Nov 2, 2023

Looks good 👍
Can you implement this in all these places?
pages/account/manage/milestone/[[...data]].js
pages/account/manage/link/[[...data]].js
pages/account/manage/event/[[...data]].js
pages/account/manage/repos.js
pages/account/manage/profile.js
pages/account/manage/premium.js

@malay44
Copy link
Contributor Author

malay44 commented Nov 3, 2023

Screenshot 2023-11-03 at 9 23 09 AM

Navbar text is coming on top of the notification. I tried to adjust the z index of the notification, but there was no change.

@sital002
Copy link
Member

sital002 commented Nov 3, 2023

Navbar text is coming on top of the notification. I tried to adjust the z index of the notification, but there was no change.

There is already a existing issue related to this #9618

@malay44 malay44 requested a review from sital002 November 3, 2023 09:44
@malay44 malay44 marked this pull request as ready for review November 7, 2023 02:53
@eddiejaoude
Copy link
Member

Looks good 👍 please can you resolve conflicts

@github-actions github-actions bot added invalid Inappropriate or invalid for Hacktoberfest contributions ✍ chore: profile This label will be added on PR with request of Data json addition tests CI/CD dependencies Pull requests that update a dependency file 📖 docs Documentation pages models labels Nov 9, 2023
@github-actions github-actions bot removed invalid Inappropriate or invalid for Hacktoberfest contributions ✍ chore: profile This label will be added on PR with request of Data json addition tests CI/CD dependencies Pull requests that update a dependency file 📖 docs Documentation pages models labels Nov 9, 2023
@malay44
Copy link
Contributor Author

malay44 commented Nov 17, 2023

Hello, I've addressed the merge conflicts mentioned earlier. Feel free to propose any additional modifications or optimizations. 😊

sital002
sital002 reviewed Nov 17, 2023
View reviewed changes
Copy link
Member

@sital002 sital002 left a comment

Choose a reason for hiding this comment

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

Sorry for the late reply, I have left some inline comments.

eddiejaoude
eddiejaoude approved these changes Jan 12, 2024
View reviewed changes
Copy link
Member

@eddiejaoude eddiejaoude left a comment

Choose a reason for hiding this comment

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

Thank you 👍

@eddiejaoude eddiejaoude merged commit 59da077 into EddieHubCommunity:main Jan 12, 2024
13 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sital002 sital002 sital002 left review comments

@eddiejaoude eddiejaoude eddiejaoude approved these changes

@SaraJaoude SaraJaoude Awaiting requested review from SaraJaoude

Assignees
No one assigned
Labels
issue linked Pull Request has issue linked
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] <adds multiple milestones>
3 participants
@malay44 @eddiejaoude @sital002