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
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
b4a9a55
feat: add useThrottle
Silence-dream Sep 27, 2022
2d69cdb
feat: submit button adds the throttling
Silence-dream Sep 28, 2022
420861e
feat(route): add publish and offline button loading
Silence-dream Sep 28, 2022
59ebb8e
feat: hook submit loading
Silence-dream Sep 29, 2022
73233ed
feat: add delete button loading
Silence-dream Sep 29, 2022
4d99636
docs: add license
Silence-dream Sep 29, 2022
9d86b19
Merge branch 'master' into feat-loading-state
Silence-dream Oct 4, 2022
10f4af0
Merge branch 'feat-loading-state' of github.com:Silence-dream/apisix-…
Silence-dream Oct 4, 2022
2a40b32
test(route): correct the test sequence
Silence-dream Oct 4, 2022
e0e284b
test(route): click on the delay
Silence-dream Oct 5, 2022
15e683d
test(route): add custom redirect delay
Silence-dream Oct 7, 2022
9d4edfe
test(route): increase delay
Silence-dream Oct 8, 2022
95cb478
test(route): increase delay
Silence-dream Oct 8, 2022
c4b2bbf
test(route):remove force
Silence-dream Oct 8, 2022
5925816
test(route): increase delay
Silence-dream Oct 8, 2022
f2d7467
test(route): increase delay
Silence-dream Oct 8, 2022
478e4f0
test(upstream): increase delay
Silence-dream Oct 8, 2022
e4b47b4
test(route): increase delay
Silence-dream Oct 9, 2022
f2e3aa2
test: revert
Silence-dream Oct 10, 2022
4570580
test: increase delay
Silence-dream Oct 10, 2022
c489c23
test: increase delay
Silence-dream Oct 10, 2022
27d3cf9
test: revert
Silence-dream Oct 10, 2022
fd8796e
test: fix can't find dom
Silence-dream Oct 10, 2022
96bc692
test: revert
Silence-dream Oct 11, 2022
997d5e6
test: revert
Silence-dream Oct 11, 2022
61de3c6
test: revert
Silence-dream Oct 11, 2022
d97b68a
Merge branch 'feat-loading-state' of github.com:Silence-dream/apisix-…
Silence-dream Oct 11, 2022
4aa28bf
test: revert
Silence-dream Oct 11, 2022
f8e901b
test: increase delay
Silence-dream Oct 11, 2022
d7e2bf1
test: increase delay
Silence-dream Oct 11, 2022
f1e0ae6
test: add deletion interval
Silence-dream Oct 11, 2022
4b0cb0a
test: revert
Silence-dream Oct 11, 2022
4cfbc70
feat: remove custom hooks and add ahooks
Silence-dream Oct 13, 2022
269ae88
fix: enable manual execution
Silence-dream Oct 13, 2022
1a27211
refactor: rename
Silence-dream Oct 17, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions web/cypress/e2e/route/batch-delete-route.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
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

.then(() => {
cy.get(selector.labels_0_labelKey).type(`label${i}`);
cy.get(selector.labels_0_labelValue).type(`value${i}`);
cy.contains('Confirm').click();
cy.contains('Confirm').click({ force: true });
});
});

cy.contains('button', 'Next').should('not.be.disabled').click();
cy.get('.ant-row').contains('Next').click({ force: true });

cy.get(selector.nodes_0_host).type(data.host2, {
timeout,
});
cy.get(selector.nodes_0_port).type(data.port);
cy.get(selector.nodes_0_weight).type(data.weight);

cy.get('.ant-row').contains('Next').click();
cy.get('.ant-row').contains('Next').click();
cy.get('.ant-row').contains('Submit').click();
// cy.contains('button', 'Next').should('not.be.disabled').click();

cy.contains(data.submitSuccess);
cy.contains('Goto List').click();
cy.url().should('contains', 'routes/list');
Expand All @@ -115,33 +117,33 @@ context('Create and Batch Deletion Routes', () => {
it('should batch delete the name of the route', function () {
cy.contains('Route').click();
const cases = [
[1, 0, 2], //full match
[1, 0, 2], // full match
[0, 1, 2], // partial match
[0, 1, 2], //none match
[0, 1, 2], // none match
];
const prefix = 'test';
cy.wrap([0, 2, 'x']).each(($n, i) => {
cy.get(selector.nameSearchInput).clear().type(`${prefix}${$n}`);
cy.contains('Search').click();
cy.wrap(cases[i]).each(($n) => {
cy.contains(`${prefix}${$n}`).should('not.exist');
cy.wrap(cases[i]).each(($m) => {
cy.contains(`${prefix}${$m}`).should('not.exist');
});
});
});

it('should batch delete the path of the route', function () {
cy.contains('Route').click();
const cases = [
[1, 0, 2], //full match
[1, 0, 2], // full match
[0, 1, 2], // partial match
[0, 1, 2], //none match
[0, 1, 2], // none match
];
const prefix = '/get';
cy.wrap([0, 2, 'x']).each(($n, i) => {
cy.get(selector.nameSearchInput).clear().type(`${prefix}${$n}`);
cy.contains('Search').click();
cy.wrap(cases[i]).each(($n) => {
cy.contains(`${prefix}${$n}`).should('not.exist');
cy.wrap(cases[i]).each(($m) => {
cy.contains(`${prefix}${$m}`).should('not.exist');
});
});
});
Expand Down
2 changes: 1 addition & 1 deletion web/cypress/e2e/route/create-route-both-use-uri-uris.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Member

Choose a reason for hiding this comment

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

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

cy.contains('Next').click();
cy.get(selector.name).type(data.name);
cy.get(selector.description).type(data.description);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,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();
Copy link
Member

Choose a reason for hiding this comment

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

ditto

cy.contains('Next').click();
cy.get(selector.name).type(data.routeName);

// All Of Operational Character Should Exist And Can be Created
Expand Down Expand Up @@ -162,7 +162,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(2000);
cy.contains('Next').click();
cy.wait(2000);
cy.contains('Submit').click();
cy.contains(data.submitSuccess);
cy.contains('Goto List').click();
Expand All @@ -185,7 +187,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(2000);
cy.contains('Next').click();
cy.wait(2000);
cy.contains('Submit').click();
cy.contains(data.submitSuccess);
cy.contains('Goto List').click();
Expand Down
12 changes: 9 additions & 3 deletions web/src/components/ActionBar/ActionBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ import React from 'react';
import type { CSSProperties } from 'react';
import { Row, Col, Button } from 'antd';
import { useIntl } from 'umi';
import useThrottle from '@/hooks/useThrottle';

type Props = {
step: number;
lastStep: number;
onChange: (nextStep: number) => void;
withResultView?: boolean;
loading?: boolean;
};

const style: CSSProperties = {
Expand All @@ -37,9 +39,13 @@ const style: CSSProperties = {
width: '100%',
};

const ActionBar: React.FC<Props> = ({ step, lastStep, onChange, withResultView }) => {
const ActionBar: React.FC<Props> = ({ step, lastStep, onChange, withResultView, loading }) => {
const { formatMessage } = useIntl();

const { fn: onChangeStep } = useThrottle((n: number) => {
onChange(step + n);
});

if (step > lastStep && !withResultView) {
onChange(lastStep);
return null;
Expand All @@ -54,12 +60,12 @@ const ActionBar: React.FC<Props> = ({ step, lastStep, onChange, withResultView }
<div style={style}>
<Row gutter={10} justify="end">
<Col>
<Button type="primary" onClick={() => onChange(step - 1)} disabled={step === 1}>
<Button type="primary" onClick={() => onChangeStep(-1)} disabled={step === 1}>
{formatMessage({ id: 'component.actionbar.button.preStep' })}
</Button>
</Col>
<Col>
<Button type="primary" onClick={() => onChange(step + 1)}>
<Button type="primary" onClick={() => onChangeStep(1)} loading={loading}>
{step < lastStep
? formatMessage({ id: 'component.actionbar.button.nextStep' })
: formatMessage({ id: 'component.global.submit' })}
Expand Down
25 changes: 25 additions & 0 deletions web/src/hooks/useLatest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can add ahooks to this library directly, without having to implement these hooks separately, what do you think? @guoqqqi @Baoyuantop @SkyeYoung

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think so. Just use the ahooks tested and used by many people. @guoqqqi @Silence-dream

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { useRef } from 'react';

function useLatest<T>(value: T) {
const ref = useRef(value);
ref.current = value;

return ref;
}
export default useLatest;
46 changes: 46 additions & 0 deletions web/src/hooks/useRequest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { useCallback, useState } from 'react';

export default function useRequest<T, Y extends any[]>(requestFn: any) {
const [loading, setLoading] = useState(false);

const [data, setData] = useState<T>();

const [err, setErr] = useState();

const fn = useCallback(async (...params: Y) => {
setLoading(true);
let res;
try {
res = await requestFn(...params);
setData(res);
} catch (error) {
// @ts-ignore
setErr(error);
}
setLoading(false);
return res;
}, []);

return {
fn,
data,
err,
loading,
};
}
58 changes: 58 additions & 0 deletions web/src/hooks/useThrottle.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import useLatest from '@/hooks/useLatest';
import { useMemo } from 'react';
import { throttle } from 'lodash';
import useUnmount from '@/hooks/useUnmount';

type useThrottleReturn = (...args: any) => any;

interface ThrottleOptions {
wait?: number;
leading?: boolean;
trailing?: boolean;
}

function useThrottle<T extends useThrottleReturn>(fn: T, options?: ThrottleOptions) {
const fnRef = useLatest(fn);

const wait = options?.wait ?? 1000;

const throttled = useMemo(
() =>
throttle(
(...args: any[]): ReturnType<T> => {
return fnRef.current(...args);
},
wait,
options,
),
[],
);

useUnmount(() => {
throttled.cancel();
});

return {
fn: throttled,
cancel: throttled.cancel,
flush: throttled.flush,
};
}

export default useThrottle;
31 changes: 31 additions & 0 deletions web/src/hooks/useUnmount.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import useLatest from '@/hooks/useLatest';
import { useEffect } from 'react';

const useUnmount = (fn: () => void) => {
const fnRef = useLatest(fn);

useEffect(
() => () => {
fnRef.current();
},
[],
);
};

export default useUnmount;
7 changes: 5 additions & 2 deletions web/src/pages/Consumer/Create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import PluginPage from '@/components/Plugin';
import Step1 from './components/Step1';
import Preview from './components/Preview';
import { fetchItem, create, update } from './service';
import useRequest from '@/hooks/useRequest';

const Page: React.FC = (props) => {
const [step, setStep] = useState(1);
Expand All @@ -43,10 +44,12 @@ const Page: React.FC = (props) => {
}
}, []);

const { fn: createConsumers, loading: submitLoading } = useRequest(create);

const onSubmit = () => {
const data = { ...form1.getFieldsValue(), plugins } as ConsumerModule.Entity;
const { username } = (props as any).match.params;
(username ? update(username, data) : create(data))
(username ? update(username, data) : createConsumers(data))
.then(() => {
notification.success({
message: `${
Expand Down Expand Up @@ -103,7 +106,7 @@ const Page: React.FC = (props) => {
{step === 3 && <Preview form1={form1} plugins={plugins} />}
</Card>
</PageContainer>
<ActionBar step={step} lastStep={3} onChange={onStepChange} />
<ActionBar loading={submitLoading} step={step} lastStep={3} onChange={onStepChange} />
</>
);
};
Expand Down
Loading