Hover plugin#639
Conversation
|
@dahlbyk Any news on merging this one? |
dahlbyk
left a comment
dahlbyk
left a comment
There was a problem hiding this comment.
@MichaelDeBoey I'll defer to @ryanlanciaux @joellanciaux on the decision to include another plugin, and if so what that plugin scope should be.
In the meantime, here are some thoughts on the code.
| focusOut: rowMouseLeave | ||
| }, dispatch) | ||
| ), | ||
| components.RowContainer |
There was a problem hiding this comment.
Rather than compose this with the default RowContainer, it's probably more correct for this to be a RowEnhancer (like in this example story).
| }, | ||
| (dispatch, {griddleKey}) => bindActionCreators({ | ||
| focusIn: rowMouseEnter.bind(this, griddleKey), | ||
| focusOut: rowMouseLeave |
There was a problem hiding this comment.
As of #679, if these were onMouseEnter/onMouseLeave they'd be passed through correctly in Row.
| connect( | ||
| state => { | ||
| return { | ||
| hoveredRowKey: state.get('hoveredRowKey', null) |
There was a problem hiding this comment.
I'm not entirely convinced it should be the plugin's responsibility to inject hoveredRowKey into Cell (or anywhere). It seems to be an implementation detail of this demo. I'm more inclined to avoid customizing Row altogether, letting ButtonsColumn be connected instead:
const ButtonsColumn = connect(
state => ({ hoveredRowKey: state.get('hoveredRowKey', null) })
)(
({griddleKey,hoveredRowKey}) =>
(griddleKey === hoveredRowKey) && <button>Edit</button> : null
);
|
@yoursdearboy Any news on addressing @dahlbyk's comments? 🙂 |
|
@MichaelDeBoey honestly, I would just copy the relevant bits into your own plugin. There's not all that much to wiring up |
|
@dahlbyk Yeah I had a few problems doing so, so was hoping the plugin could help 😕 Will try and make some issues together with a codesandbox, so you could maybe point me in the right direction if you want 🙂 |
|
Glad to help as time allows |
3 participants
Griddle major version
1.3.0
Changes proposed in this pull request
Added a plugin that provide each
CellwithhoveredRowKeyproperty which is equal togriddleKeyof hovered row.Why these changes are made
Don't know if this repo is a proper place to contribute plugins (at least I see plugins directory).
We use it to show toolbar for records in rows (see storybook demo).
Are there tests?
Storybook story only.