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: Adding a Loading state to buttons #2630

Merged
merged 35 commits into from
Oct 18, 2022

Conversation

Silence-dream
Copy link
Contributor

@Silence-dream Silence-dream commented Sep 29, 2022

Please answer these questions before submitting a pull request, or your PR will get closed.

Why submit this pull request?

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What changes will this PR take into?

Add a global custom Hook to change the state of the button to Loading when clicking on a button in the Create, Submit, Delete etc. category and restore the state when the request interface is successful.

The specific scope of modification :
in routes submit,publish,offline button
in upstream submit and delete button
in service submit and delete button
in consumer submit and delete button
in proto submit and delete button
in plugin submit and delete button
in ssl submit and delete button

Related issues

fix/resolve #2625

Checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@Silence-dream Silence-dream changed the title Feat loading state feat: Adding a Loading state to buttons Sep 29, 2022
@guoqqqi
Copy link
Member

guoqqqi commented Sep 30, 2022

also @SkyeYoung PTAL

@codecov-commenter
Copy link

codecov-commenter commented Sep 30, 2022

Codecov Report

Merging #2630 (a1b4c2d) into master (e68b3aa) will increase coverage by 1.12%.
The diff coverage is 84.80%.

❗ Current head a1b4c2d differs from pull request most recent head 4b0cb0a. Consider uploading reports for the commit 4b0cb0a to get more accurate results

@@            Coverage Diff             @@
##           master    #2630      +/-   ##
==========================================
+ Coverage   68.29%   69.42%   +1.12%     
==========================================
  Files         139      140       +1     
  Lines        3694     3702       +8     
  Branches      872      869       -3     
==========================================
+ Hits         2523     2570      +47     
+ Misses       1171     1132      -39     
Flag Coverage Δ
frontend-e2e-test 69.42% <84.80%> (+1.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
web/src/pages/Route/List.tsx 73.05% <15.38%> (+14.30%) ⬆️
web/src/pages/Route/Create.tsx 81.39% <75.00%> (-1.94%) ⬇️
web/src/pages/SSL/Create.tsx 81.81% <75.00%> (-2.40%) ⬇️
web/src/pages/Service/Create.tsx 93.33% <75.00%> (-1.31%) ⬇️
web/src/pages/Upstream/Create.tsx 88.88% <81.81%> (-3.42%) ⬇️
web/src/hooks/useRequest.ts 92.30% <92.30%> (ø)
web/src/components/ActionBar/ActionBar.tsx 84.61% <100.00%> (+7.69%) ⬆️
web/src/hooks/useLatest.ts 100.00% <100.00%> (ø)
web/src/hooks/useThrottle.ts 100.00% <100.00%> (ø)
web/src/hooks/useUnmount.ts 100.00% <100.00%> (ø)
... and 37 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Baoyuantop
Copy link
Contributor

Hi @Silence-dream, excellent work. Can you get the CI passed? Feel free to ask if you need any help!

@Silence-dream
Copy link
Contributor Author

Hi @Silence-dream, excellent work. Can you get the CI passed? Feel free to ask if you need any help!

I have no experience with Cypress, can you give me some ideas?

@SkyeYoung
Copy link
Member

@Silence-dream Just read the documentation in https://www.cypress.io/, which is very friendly. 😸

@guoqqqi
Copy link
Member

guoqqqi commented Oct 1, 2022

Hi @Silence-dream, excellent work. Can you get the CI passed? Feel free to ask if you need any help!

I have no experience with Cypress, can you give me some ideas?

Hello, You can read some of the older test cases and if you have any questions about starting a test environment, you can refer to:

https://github.com/apache/apisix-dashboard/blob/master/docs/en/latest/develop.md
https://github.com/apache/apisix-dashboard/blob/master/docs/en/latest/front-end-e2e.md

cy.get(selector.drawerBody).within(($drawer) => {
cy.wrap($drawer)
.contains('button', 'Add')
.should('not.be.disabled')
.click()
.click({ force: true })
Copy link
Member

Choose a reason for hiding this comment

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

why does it need to add force here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

make click work

@guoqqqi
Copy link
Member

guoqqqi commented Oct 11, 2022

Hello @Silence-dream After the CI is run, you can download the video to help determine the cause of the bug.
Previous test record:
https://github.com/apache/apisix-dashboard/actions/runs/3217789354
image

@Silence-dream
Copy link
Contributor Author

Hello @Silence-dream After the CI is run, you can download the video to help determine the cause of the bug.

Thank you very much! It was very useful!

@guoqqqi
Copy link
Member

guoqqqi commented Oct 11, 2022

NOTE: Since you are contributing to CI for the first time, you will need a Committer to trigger the CI to run.

@Silence-dream Silence-dream marked this pull request as ready for review October 11, 2022 09:01
web/src/pages/Consumer/List.tsx Outdated Show resolved Hide resolved
web/src/pages/Plugin/List.tsx Outdated Show resolved Hide resolved
web/src/pages/Route/Create.tsx Outdated Show resolved Hide resolved
web/src/pages/Route/List.tsx Outdated Show resolved Hide resolved
web/src/pages/Route/List.tsx Outdated Show resolved Hide resolved
web/src/pages/Service/Create.tsx Outdated Show resolved Hide resolved
web/src/pages/Upstream/Create.tsx Outdated Show resolved Hide resolved
@guoqqqi guoqqqi requested review from SkyeYoung, oil-oil and Baoyuantop and removed request for SkyeYoung and oil-oil October 17, 2022 03:35
web/src/pages/Consumer/List.tsx Outdated Show resolved Hide resolved
Copy link
Member

@guoqqqi guoqqqi left a comment

Choose a reason for hiding this comment

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

LGTM, also cc @LiteSun @Baoyuantop PTAL

@guoqqqi guoqqqi merged commit 7cbdc48 into apache:master Oct 18, 2022
hongbinhsu added a commit to fitphp/apix-dashboard that referenced this pull request Nov 7, 2022
* upstream/master:
  fix: change default CSP value (apache#2601)
  fix: ant-table unable to request (apache#2641)
  fix: plugin_config missing on service exist (apache#2657)
  feat: add etcd store auto re-initialize (apache#2650)
  feat: add login filter of OpenID-Connect (apache#2608)
  feat:Configure plug-ins to support this feature (apache#2647)
  feat: Adding a Loading state to buttons (apache#2630)
  feat: dashboard support windows (apache#2619)
  Feat: add tip and preset model for plugin editor, improve e2e stability (apache#2581)
  docs: add Slack invitation link badge (apache#2617)

# Conflicts:
#	.github/workflows/backend-cli-test.yml
#	Dockerfile
#	api/test/shell/cli_test.sh
#	web/src/components/Footer/index.tsx
#	web/src/components/RightContent/index.tsx
#	web/src/pages/ServerInfo/List.tsx
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.

[feat]: Adding a Loading state to buttons
7 participants