(Fix) "Approve transaction" modal closes#1477
(Fix) "Approve transaction" modal closes#1477fernandomg merged 2 commits intofeature/#1353-xDai-compatibilityfrom
Conversation
|
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
|
Travis automatic deployment: |
francovenica
left a comment
francovenica
left a comment
There was a problem hiding this comment.
Works fine. The modal is not closing itself anymore
fernandomg
changed the title
src/routes/safe/container/index.tsx
Outdated
| const TxsTable = React.lazy(() => import('src/routes/safe/components/Transactions/TxsTable')) | ||
| const AddressBookTable = React.lazy(() => import('src/routes/safe/components/AddressBook')) | ||
|
|
||
| const featuresEqualityFn = (left, right) => { |
There was a problem hiding this comment.
hmm... why does it not complain about the types? :P
Also, I think It could be a generic util function, not only featuresEquality
There was a problem hiding this comment.
Actually, even a better solution would be removing enabledFeatures from the redux store... It can be computed only with a version number, doesn't have to be stored.
There was a problem hiding this comment.
agreed. But let's do it in a different PR once xDai is merged. I created this ticket #1479 to remove it from the store.
There was a problem hiding this comment.
Why? It's not a huge change. The amount of tech debt tickets we have is pretty high (and there's a ton of discussed, but not created ones), so I'd think before adding a new one
There was a problem hiding this comment.
const featuresEnabled = useMemo(() => enabledFeatures(safe.version), [safe.version])
literally, that's it, except for removing the calculation in fetchSafe.ts, updating types, and replacing 2 references to the selector with a function from above. It's around 10 mins of work.
There was a problem hiding this comment.
the thing is we need to release xDai on Monday. So Franco needs to test everything on xDAi and do a regression on Mainnet ASAP. I understand your point, but I would suggest working on it on Monday if everything goes well.
There was a problem hiding this comment.
- I left the inline equalityFn, as an ad-hoc solution. First, because we're going to remove this when #1479 is done Second, because this way it infers the types defined in the useSelector generics
|
Travis automatic deployment: |
5 participants
This PR closes #1474, by:
useActionhook for the featuresEnabled selector