You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/09/29 08:02:22 UTC
[GitHub] [apisix-dashboard] Silence-dream opened a new pull request, #2630: Feat loading state
Silence-dream opened a new pull request, #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630
Please answer these questions before submitting a pull request, **or your PR will get closed**.
**Why submit this pull request?**
- [ ] Bugfix
- [x] 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:**
- [x] 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?
- [x] Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Silence-dream commented on a diff in pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
Silence-dream commented on code in PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#discussion_r989982466
##########
web/cypress/e2e/route/batch-delete-route.cy.js:
##########
@@ -63,36 +63,38 @@ context('Create and Batch Deletion Routes', () => {
for (let i = 0; i < 3; i += 1) {
cy.wait(timeout);
cy.get('.ant-pro-table-list-toolbar-right').contains('Create').click();
- cy.get('.ant-row').contains('Next').click().click();
cy.get(selector.name).type(`test${i}`);
cy.get(selector.description).type(`desc${i}`);
cy.get(selector.hosts_0).type(data.host1);
cy.get(selector.uris_0).clear().type(`/get${i}`);
-
// config label
cy.contains('Manage').click();
-
+ // eslint-disable-next-line @typescript-eslint/no-loop-func
cy.get(selector.drawerBody).within(($drawer) => {
cy.wrap($drawer)
.contains('button', 'Add')
.should('not.be.disabled')
- .click()
+ .click({ force: true })
Review Comment:
make click work
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Silence-dream commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
Silence-dream commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1263289064
> 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] SkyeYoung commented on a diff in pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
SkyeYoung commented on code in PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#discussion_r990901470
##########
web/src/hooks/useLatest.ts:
##########
@@ -0,0 +1,25 @@
+/*
Review Comment:
Yes, I think so. Just use the `ahooks` tested and used by many people. @guoqqqi @Silence-dream
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi commented on a diff in pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
guoqqqi commented on code in PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#discussion_r996583502
##########
web/src/pages/Consumer/List.tsx:
##########
@@ -38,7 +38,7 @@ const Page: React.FC = () => {
const [id, setId] = useState('');
const [editorMode, setEditorMode] = useState<'create' | 'update'>('create');
const { paginationConfig, savePageList, checkPageList } = usePagination();
-
+ const [deleteLoading, setDeleteLoading] = useState('');
Review Comment:
`deleteLoading` bad Variable name
##########
web/src/pages/Plugin/List.tsx:
##########
@@ -36,6 +36,7 @@ const Page: React.FC = () => {
const [pluginList, setPluginList] = useState<PluginComponent.Meta[]>([]);
const [name, setName] = useState('');
const { paginationConfig, savePageList, checkPageList } = usePagination();
+ const [deleteLoading, setDeleteLoading] = useState('');
Review Comment:
ditto
##########
web/src/pages/Route/List.tsx:
##########
@@ -108,18 +109,28 @@ const Page: React.FC = () => {
checkPageList(ref);
};
- const handlePublishOffline = (rid: string, status: RouteModule.RouteStatus) => {
- updateRouteStatus(rid, status).then(() => {
- const actionName = status
- ? formatMessage({ id: 'page.route.publish' })
- : formatMessage({ id: 'page.route.offline' });
- handleTableActionSuccessResponse(
- `${actionName} ${formatMessage({
- id: 'menu.routes',
- })} ${formatMessage({ id: 'component.status.success' })}`,
- );
- });
- };
+ const [publishOfflineLoading, setPublishOfflineLoading] = useState<string>('');
Review Comment:
`loading` makes it read like a `boolean`
##########
web/src/pages/Service/Create.tsx:
##########
@@ -62,8 +62,9 @@ const Page: React.FC = (props) => {
});
}
}, []);
-
+ const [submitLoading, setSubmitLoading] = useState(false);
Review Comment:
Usually, we put the Hook at the top of the function scope
##########
web/cypress/e2e/route/create-route-with-search-service-and-set-priority.cy.js:
##########
@@ -171,7 +172,7 @@ context('Create Route with search service name', () => {
cy.contains(data.serviceName).siblings().contains('Delete').click();
cy.contains('button', 'Confirm').click();
cy.get(selector.notification).should('contain', data.deleteServiceSuccess);
-
+ cy.wait(4000);
Review Comment:
You can click here to remove the tip in the upper right corner, we should try not to waste time by waiting as much as possible:
```
cy.get('.ant-notification-close-icon').click().should('not.exist');
```
##########
web/src/pages/Route/List.tsx:
##########
@@ -59,6 +59,7 @@ import { DebugDrawView } from './components/DebugViews';
import { RawDataEditor } from '@/components/RawDataEditor';
import { EXPORT_FILE_MIME_TYPE_SUPPORTED } from './constants';
import DataLoaderImport from '@/pages/Route/components/DataLoader/Import';
+import { useThrottleFn } from 'ahooks';
Review Comment:
move this line to 45
##########
web/src/pages/Route/Create.tsx:
##########
@@ -32,6 +32,7 @@ import CreateStep4 from './components/CreateStep4';
import { DEFAULT_STEP_1_DATA, DEFAULT_STEP_3_DATA } from './constants';
import ResultView from './components/ResultView';
import styles from './Create.less';
+import { useRequest } from 'ahooks';
Review Comment:
move this line to 22
##########
web/src/pages/Upstream/Create.tsx:
##########
@@ -48,8 +48,10 @@ const Page: React.FC = (props) => {
});
}
}, []);
+ const [submitLoading, setSubmitLoading] = useState(false);
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Silence-dream commented on a diff in pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
Silence-dream commented on code in PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#discussion_r990908834
##########
web/src/hooks/useLatest.ts:
##########
@@ -0,0 +1,25 @@
+/*
Review Comment:
>
So I remove custom hooks and add ahooks?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-commenter commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1262961285
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/2630?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#2630](https://codecov.io/gh/apache/apisix-dashboard/pull/2630?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4d99636) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/b777d99be72bfaca96561047218d36aa213da952?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b777d99) will **decrease** coverage by `1.54%`.
> The diff coverage is `95.74%`.
```diff
@@ Coverage Diff @@
## master #2630 +/- ##
==========================================
- Coverage 68.50% 66.96% -1.55%
==========================================
Files 134 41 -93
Lines 3553 1011 -2542
Branches 867 268 -599
==========================================
- Hits 2434 677 -1757
+ Misses 1119 334 -785
```
| Flag | Coverage Δ | |
|---|---|---|
| frontend-e2e-test | `66.96% <95.74%> (-1.55%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/2630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [web/src/components/ActionBar/ActionBar.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2630/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9jb21wb25lbnRzL0FjdGlvbkJhci9BY3Rpb25CYXIudHN4) | `73.33% <80.00%> (-11.29%)` | :arrow_down: |
| [web/src/hooks/useRequest.ts](https://codecov.io/gh/apache/apisix-dashboard/pull/2630/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9ob29rcy91c2VSZXF1ZXN0LnRz) | `92.30% <92.30%> (ø)` | |
| [web/src/hooks/useLatest.ts](https://codecov.io/gh/apache/apisix-dashboard/pull/2630/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9ob29rcy91c2VMYXRlc3QudHM=) | `100.00% <100.00%> (ø)` | |
| [web/src/hooks/useThrottle.ts](https://codecov.io/gh/apache/apisix-dashboard/pull/2630/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9ob29rcy91c2VUaHJvdHRsZS50cw==) | `100.00% <100.00%> (ø)` | |
| [web/src/hooks/useUnmount.ts](https://codecov.io/gh/apache/apisix-dashboard/pull/2630/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9ob29rcy91c2VVbm1vdW50LnRz) | `100.00% <100.00%> (ø)` | |
| [web/src/pages/Consumer/Create.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2630/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9wYWdlcy9Db25zdW1lci9DcmVhdGUudHN4) | `77.50% <100.00%> (+0.57%)` | :arrow_up: |
| [web/src/pages/Consumer/List.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2630/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9wYWdlcy9Db25zdW1lci9MaXN0LnRzeA==) | `73.17% <100.00%> (-18.50%)` | :arrow_down: |
| [web/src/helpers.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2630/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9oZWxwZXJzLnRzeA==) | `19.67% <0.00%> (-50.82%)` | :arrow_down: |
| [web/src/components/RightContent/AvatarDropdown.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2630/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9jb21wb25lbnRzL1JpZ2h0Q29udGVudC9BdmF0YXJEcm9wZG93bi50c3g=) | `50.00% <0.00%> (-32.15%)` | :arrow_down: |
| [web/src/components/PanelSection/index.tsx](https://codecov.io/gh/apache/apisix-dashboard/pull/2630/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-d2ViL3NyYy9jb21wb25lbnRzL1BhbmVsU2VjdGlvbi9pbmRleC50c3g=) | `75.00% <0.00%> (-25.00%)` | :arrow_down: |
| ... and [109 more](https://codecov.io/gh/apache/apisix-dashboard/pull/2630/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Baoyuantop commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
Baoyuantop commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1263009522
Hi @Silence-dream, excellent work. Can you get the CI passed? Feel free to ask if you need any help!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Silence-dream commented on a diff in pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
Silence-dream commented on code in PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#discussion_r990908896
##########
web/src/hooks/useLatest.ts:
##########
@@ -0,0 +1,25 @@
+/*
Review Comment:
> Yes, I think so. Just use the `ahooks` tested and used by many people. @guoqqqi @Silence-dream
> Yes, I think so. Just use the `ahooks` tested and used by many people. @guoqqqi @Silence-dream
So I remove custom hooks and add ahooks?
##########
web/src/hooks/useLatest.ts:
##########
@@ -0,0 +1,25 @@
+/*
Review Comment:
> Yes, I think so. Just use the `ahooks` tested and used by many people. @guoqqqi @Silence-dream
So I remove custom hooks and add ahooks?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Silence-dream commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
Silence-dream commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1274056127
> 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!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] oil-oil commented on a diff in pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
oil-oil commented on code in PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#discussion_r994258717
##########
web/src/hooks/useLatest.ts:
##########
@@ -0,0 +1,25 @@
+/*
Review Comment:
> > Yes, I think so. Just use the `ahooks` tested and used by many people. @guoqqqi @Silence-dream
>
> So I remove custom hooks and add ahooks?
Yes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Silence-dream commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
Silence-dream commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1271683148
Some errors I can't reproduce
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi merged pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
guoqqqi merged PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] SkyeYoung commented on a diff in pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
SkyeYoung commented on code in PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#discussion_r996635739
##########
web/src/pages/Consumer/List.tsx:
##########
@@ -38,7 +38,7 @@ const Page: React.FC = () => {
const [id, setId] = useState('');
const [editorMode, setEditorMode] = useState<'create' | 'update'>('create');
const { paginationConfig, savePageList, checkPageList } = usePagination();
-
+ const [deleteLoading, setDeleteLoading] = useState('');
Review Comment:
Ye, agree. What does `deleteLoading` means? If it needs to be explained, u should try to rename a better name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi commented on a diff in pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
guoqqqi commented on code in PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#discussion_r990899096
##########
web/cypress/e2e/route/create-route-with-advanced-matching-conditions.cy.js:
##########
@@ -70,7 +72,7 @@ context('Create Route with advanced matching conditions', () => {
it('should create route with advanced matching conditions', function () {
cy.visit('/routes/list');
cy.contains('Create').click();
- cy.contains('Next').click().click();
Review Comment:
ditto
##########
web/cypress/e2e/route/create-route-with-advanced-matching-conditions.cy.js:
##########
@@ -185,7 +189,9 @@ context('Create Route with advanced matching conditions', () => {
cy.contains('Next').click();
cy.get(selector.nodes_0_port).focus();
cy.contains('Next').click();
+ cy.wait(timeout * 2);
cy.contains('Next').click();
+ cy.wait(timeout * 2);
Review Comment:
Remove all new wait
##########
web/cypress/e2e/route/search-route.cy.js:
##########
@@ -82,7 +82,9 @@ context('Create and Search Route', () => {
cy.wait(timeout);
for (let i = 0; i < 3; i += 1) {
cy.contains('Create').click();
- cy.contains('Next').click().click();
+ cy.contains('Next').click();
Review Comment:
Remove all new wait
##########
web/cypress/e2e/route/create-route-with-advanced-matching-conditions.cy.js:
##########
@@ -185,7 +189,9 @@ context('Create Route with advanced matching conditions', () => {
cy.contains('Next').click();
cy.get(selector.nodes_0_port).focus();
cy.contains('Next').click();
+ cy.wait(timeout * 2);
Review Comment:
ditto
##########
web/cypress/e2e/route/create-route-with-search-service-and-set-priority.cy.js:
##########
@@ -109,7 +110,9 @@ context('Create Route with search service name', () => {
cy.visit('/');
cy.contains('Route').click();
cy.contains('Create').click();
- cy.contains('Next').click().click();
Review Comment:
ditto
##########
web/cypress/e2e/route/create-route-with-chash-upstream.cy.js:
##########
@@ -102,6 +102,7 @@ context('Create and Edit Route With Custom CHash Key Upstream', () => {
cy.get(selector.chash_key).should('value', data.custom_key);
cy.get(selector.chash_key).clear().type(data.new_key);
cy.contains('Next').click();
+ cy.wait(1000);
Review Comment:
Remove all new wait
##########
web/cypress/e2e/route/create-route-with-advanced-matching-conditions.cy.js:
##########
@@ -162,7 +164,9 @@ context('Create Route with advanced matching conditions', () => {
cy.contains('Next').click();
cy.get(selector.nodes_0_port).focus();
cy.contains('Next').click();
+ cy.wait(timeout * 2);
Review Comment:
Remove all new wait
##########
web/cypress/e2e/route/create-route-both-use-uri-uris.cy.js:
##########
@@ -72,7 +72,7 @@ context('Create Route Both use uri and uris', () => {
cy.contains('Route').click();
cy.get(selector.empty).should('be.visible');
cy.contains('Create').click();
- cy.contains('Next').click().click();
Review Comment:
The two clicks here are to ensure that the rendering of the page is finished, and this is a point that can be optimized.
Please try to restore it first
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] SkyeYoung commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
SkyeYoung commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1263291895
@Silence-dream Just read the documentation in https://www.cypress.io/, which is very friendly. 😸
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
guoqqqi commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1264381000
> > 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](https://github.com/apache/apisix-dashboard/blob/master/web/cypress/e2e/route/batch-delete-route.cy.js) 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] LiteSun commented on a diff in pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
LiteSun commented on code in PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#discussion_r989753927
##########
web/cypress/e2e/route/batch-delete-route.cy.js:
##########
@@ -63,36 +63,38 @@ context('Create and Batch Deletion Routes', () => {
for (let i = 0; i < 3; i += 1) {
cy.wait(timeout);
cy.get('.ant-pro-table-list-toolbar-right').contains('Create').click();
- cy.get('.ant-row').contains('Next').click().click();
cy.get(selector.name).type(`test${i}`);
cy.get(selector.description).type(`desc${i}`);
cy.get(selector.hosts_0).type(data.host1);
cy.get(selector.uris_0).clear().type(`/get${i}`);
-
// config label
cy.contains('Manage').click();
-
+ // eslint-disable-next-line @typescript-eslint/no-loop-func
cy.get(selector.drawerBody).within(($drawer) => {
cy.wrap($drawer)
.contains('button', 'Add')
.should('not.be.disabled')
- .click()
+ .click({ force: true })
Review Comment:
why does it need to add force here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Silence-dream commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
Silence-dream commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1272276146
https://github.com/apache/apisix-dashboard/actions/runs/3209405460/jobs/5246820969
pluginTemplate-create-plugin-template-with-route.cy.js
My execution is normal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
guoqqqi commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1274054073
Hello @Silence-dream After the CI is run, you can download the video to help determine the cause of the bug.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
guoqqqi commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1262958030
also @SkyeYoung PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
guoqqqi commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1274057049
NOTE: Since you are contributing to CI for the first time, you will need a Committer to trigger the CI to run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] guoqqqi commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
guoqqqi commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1272755497
Hi, @Silence-dream Please delete all new wait and resume the previous successive click attempts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Silence-dream commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
Silence-dream commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1272440478
I don't understand why ci would fail, my execution was successful
In the previous implementation the results were successful
https://github.com/apache/apisix-dashboard/actions/runs/3209938717/jobs/5247221587
https://github.com/apache/apisix-dashboard/actions/runs/3210584308/jobs/5251519348
before
after
before
after
can you help me? @guoqqqi @LiteSun
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] oil-oil commented on a diff in pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
oil-oil commented on code in PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#discussion_r990740182
##########
web/src/hooks/useLatest.ts:
##########
@@ -0,0 +1,25 @@
+/*
Review Comment:
I think we can add `ahooks` to this library directly, without having to implement these hooks separately, what do you think? @guoqqqi @Baoyuantop @SkyeYoung
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Silence-dream commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
Silence-dream commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1272750285
Different results for the same commit, unbelievable
https://github.com/apache/apisix-dashboard/actions/runs/3213197938/jobs/5258560129
https://github.com/Silence-dream/apisix-dashboard/actions/runs/3213197898/jobs/5252694233
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] SkyeYoung commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
SkyeYoung commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1272759497
> Hi, @Silence-dream Please delete all new wait and resume the previous successive click attempts
@Silence-dream May I ask why there are so many new `wait`s? Are there any other solutions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] Silence-dream commented on pull request #2630: feat: Adding a Loading state to buttons
Posted by GitBox <gi...@apache.org>.
Silence-dream commented on PR #2630:
URL: https://github.com/apache/apisix-dashboard/pull/2630#issuecomment-1272777474
> > Hi, @Silence-dream Please delete all new wait and resume the previous successive click attempts
> > @Silence-dream May I ask why there are so many new `wait`s? Are there any other solutions?
The wait is added to ensure that the dom is obtained, but it works on my computer, both with and without the
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org