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

Fixes #49 Reload in progress notification #122

Open
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@gabbaxx
Contributor

gabbaxx commented Dec 3, 2018

This change opens a "reload in progress" popover that dissapears when the reload is finished. Bear in mind, detecting this isn't an exact science due to various timing edge cases so the solution isn't ideal - but it serves its purpose.

@gabbaxx gabbaxx requested review from hrigner, peol and snurrfint Dec 3, 2018

@netlify

This comment has been minimized.

netlify bot commented Dec 3, 2018

Deploy preview for catwalk ready!

Built with commit 60f372c

https://deploy-preview-122--catwalk.netlify.com

Show resolved Hide resolved src/components/app.jsx Outdated
Show resolved Hide resolved src/components/app.jsx Outdated
@@ -1,6 +1,7 @@
import { useState, useEffect } from 'react';
import debounce from '../render-debouncer';
const RELOAD_IN_PROGRESS = 11000;

This comment has been minimized.

@peol

peol Dec 3, 2018

Member

Perhaps export this from the reload-in-progress interceptor file since it's used in many places?

Show resolved Hide resolved src/components/use/layout.jsx Outdated
Show resolved Hide resolved src/enigma/reload-in-progress-interceptor.js Outdated
@@ -29,7 +29,7 @@ function TableFieldWithoutState({
}) {
let classes = `table-field ${fieldData.qKeyType}`;
if (!layout) {
if (!layout || !layout.qListObject || !layout.qListObject.qDataPages[0]) {

This comment has been minimized.

@peol

peol Dec 3, 2018

Member

When would we have a layout, but not a qListObject defined?

import { useState, useEffect } from 'react';
const ERR_RELOAD_IN_PROGRESS = 11000;
let wasInReloadOnStartup;

This comment has been minimized.

@peol

peol Dec 3, 2018

Member

Not too fond of these global states here, makes it harder to test and re-use. Can we cache this based on an appid or something instead (if needed)?

return reloadInProgress;
}
function retryUsingTimeouts(request) {

This comment has been minimized.

@peol

peol Dec 3, 2018

Member

async function here should let you use await/async directly instead of having to use new Promise below.

Show resolved Hide resolved src/enigma/reload-in-progress-interceptor.js
if (setReloadInProgress) {
setReloadInProgress(true);
} else {
wasInReloadOnStartup = true;

This comment has been minimized.

@peol

peol Dec 3, 2018

Member

Wouldn't wasInReloadOnStartup only be true if it's the GetActiveDoc or OpenDoc methods?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment