Allow selection & saving of available views #2131
Conversation
|
It looks like this contributor signed our Contributor License Agreement. 👍 Many thanks, Ethcore CLA Bot |
jacogr
changed the title
| } | ||
|
|
||
| onActivate = (tab) => { | ||
| const { router } = this.context; | ||
| const activeRoute = tab.props['data-route']; |
There was a problem hiding this comment.
Why do we have to use data attributes here?
We could either:
- Attach lambda for each
Tab::onActivate(less performant?) - Wrap the Tab into another component (not sure if possible?)
I don't feel that data- attributes is the best way to do it.
There was a problem hiding this comment.
Not sure I like those options either. I remember that I spent time researching this and this seemed to be the "least hacky way" in my view :( Thinking about it again, I will try your option #1 though, it may be slightly less problematic. (Still not great)
There was a problem hiding this comment.
Fixed with #1. Not happy, but less of a bad option maybe.
| } | ||
|
|
||
| Object.keys(lsdata).forEach((id) => { | ||
| state[`${id}Visible`] = lsdata[id].active; |
There was a problem hiding this comment.
${id}Visible could be wrapped in a separate function to avoid repeating a random string in couple places.
| lsdata[id].active = this.state[`${id}Visible`]; | ||
| }); | ||
|
|
||
| window.localStorage.setItem(LS_VIEWS, JSON.stringify(lsdata)); |
There was a problem hiding this comment.
writing to localStorage may throw exceptions in certain configurations - might be better to create a Storage wrapper and handle such cases globaly.
There was a problem hiding this comment.
e.g. exception if sandboxed. Yes, not a bad idea I think we will probably start (ab)using it a bit more apart from the current tooltips, signer & views config. Should do it properly. (Going to log that seperately to split these out, since I wouldn't just want to do it for the one place and the rest don't fit in this PR)
There was a problem hiding this comment.
tomusdrw
added
A5-grumble 🔥
jacogr
added
A0-pleasereview 🤓
Functionality in action - https://youtu.be/xPSrHKB3VqU
Needed for non-default features. #2130