Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fix Token Reg Dapp issues in Firefox #4489

Merged
merged 4 commits into from
Feb 9, 2017
Merged

Fix Token Reg Dapp issues in Firefox #4489

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1918ebd
Fix overflow issues in Firefox (#4348)
ngotchac Feb 9, 2017
eaec4ee
Fix wrong Promise inferance
ngotchac Feb 9, 2017
6ffbdab
Add new Componennt for Token Images (#4496)
ngotchac Feb 9, 2017
35933dc
Revert "Add new Componennt for Token Images (#4496)"
ngotchac Feb 9, 2017
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
22 changes: 11 additions & 11 deletions js/src/dapps/tokenreg/Inputs/Text/input-text.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,22 +105,22 @@ export default class InputText extends Component {
const { validationType, contract } = this.props;
const validation = validate(value, validationType, contract);

if (validation instanceof Promise) {
const loadingTimeout = setTimeout(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, this wasn't working in Firefox...

Copy link
Contributor

Choose a reason for hiding this comment

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

and we shouldn't do it anyway. there's Promise.resolve and npm i is-promise

Copy link
Contributor

Choose a reason for hiding this comment

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

why doesn't it work in firefox? it's valid JS isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't work reliably with the es6-promise shim, which I'm guessing is included in FF. (Bluebird has the same issue.)

Chrome doesn't show it since it has native Promises.

Copy link
Contributor

Choose a reason for hiding this comment

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

@gavofyork In JS, it is considered bad practice to assert instanceof in these cases. We only care about getting a thenable interface, which Promise.resolve should happily accept. See then/is-promise#6

this.setState({ disabled: true, loading: true });
}, 50);
Copy link
Contributor

Choose a reason for hiding this comment

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

Doing this always feels very hacky, i.e. you are working around something you shouldn't be doing :) Only a mustn't-grumble.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm... It's more that it's not necessary to change the state twice if the request resolves in less than 50ms, no ?


return validation
.then(validation => {
this.setValidation({
...validation,
disabled: false,
loading: false
});
return Promise.resolve(validation)
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

.then((validation) => {
clearTimeout(loadingTimeout);

event.target.focus();
this.setValidation({
...validation,
disabled: false,
loading: false
});
}

this.setValidation(validation);
event.target.focus();
});
}

onKeyDown = (event) => {
Expand Down
2 changes: 1 addition & 1 deletion js/src/dapps/tokenreg/Tokens/Token/token.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
}

.token-container {
flex: 1;
flex: 1 1 auto;
}

.full-width .token-container {
Expand Down