A few improvements to the Tabs component#797
Conversation
|
|
| editorId: v.editorId, | ||
| title, | ||
| modified, | ||
| active: EditorGroup.isActiveEditor(editorGroup, v.editorId), |
|
|
||
| let viewStyle = Style.[flexDirection(`Row)]; | ||
| let measureWidth: option(node) => int = fun | ||
| | Some(outer) => outer#measurements().width |
There was a problem hiding this comment.
I didn't realize you could use the fun keyword like this - cool!
| }; | ||
|
|
||
| let isPendingRender: option(node) => bool = fun | ||
| | Some(outer) => outer#firstChild()#firstChild()#measurements().width < 0 |
There was a problem hiding this comment.
Wow! Nice find on how to figure out if a render is pending.
I wonder if we could provide a helper API on the Revery side to make this more ergonomic?
There was a problem hiding this comment.
I hope so!
This is basically just what i tried to do, and found to return garbage data in some circumstances. It worries me a bit that such a widely used API can just return garbage like that. It kind of makes the point of using a safe language a bit moot. Should it be considered a bug in revery?
The easy fix is probably just to have measurements() return an option, have it detect MIN_INT on some measure and return None in that case. But that will still require this onDimensionsChange hack everywhere this can happen. Perhaps we could have measurements() take a callback instead, that's called when rendering is done?
There was a problem hiding this comment.
It worries me a bit that such a widely used API can just return garbage like that. It kind of makes the point of using a safe language a bit moot. Should it be considered a bug in revery?
Agree 100% ! Good observation. The garbage is a problem - we should be leveraging the language to make it safe and easy to create UI (like we do for things like hooks).
Perhaps we could have measurements() take a callback instead, that's called when rendering is done?
I like this idea!
| ( | ||
| hooks, | ||
| <View | ||
| onMouseWheel=scroll |
|
Hi @glennsl , Sorry - realized the CI was busted for this change! I'll merge up So excited to have this in 🎉 |
|
Oh - looks like I need to bring in the overflow Revery fix to it. Looking into that. |
|
Found and fixed an additional overflow issue: revery-ui/revery#599 Brought it in; I'll merge up and this should be good-to-go! 👍 |
|
Green now - thanks @glennsl for the incredible contribution! 🎉 |
* bounded mousewheel scrolling * include icon in clickable tab area * bring active tab into view when changed * simplify tabInfo * cleanup * fix unused variable warnings * Formatting
3 participants
tabInfoby addingeditorIdand removing the properties that were precomputed based on it,active,onClickandonClose.Depends on revery-ui/revery#579 to fix tabs overflowing