-
Notifications
You must be signed in to change notification settings - Fork 295
Allow to optionally only toggle if clicked on the toggler #9
Comments
@bkniffler That makes sense to me. The only problem is, we will have to push the onClick to the toggle component as well, which makes it more complicated overall. Especially when you have custom toggle and header decorators, you will have to remember to use onClick twice. What do you think? |
I think it would make sense to join header and toggle decorators, this would give us more flexibility. What do you think about that? |
Just as an example, this is how my header looks like now. It checks weather its a terminal node to insert the caret. Also, it checks for active state for bold font. And if a 'code' is present, an additional link is displayed. const {id, toggled, code, name, active, terminal} = props.node;
<div onClick={toggle} style={{...props.style.base}}>
{!terminal ? <strong style={{display: 'table-cell', width:'20px'}}>
<i className={"icon caret " + (toggled ? 'down' : 'right')} style={{position:'absolute', top:'1px'}}></i>
</strong> : <strong style={{display: 'table-cell', width:'20px'}}> </strong>}
{code ? <a onClick={activate} href="javascript:;" style={{display: 'table-cell', minWidth: '27px', paddingRight:'10px', whiteSpace: 'nowrap'}}>{code}</a> : null}
<span style={{display: 'table-cell', fontWeight: active?'bold':'initial'}}>{name}</span>
</div> |
Its a difficult one. Because a lot of times you will want to use another header, but keep the toggle the same, as a lot of people will be happy with the default toggle decorator. But the header must be changed for simple things like icons. I'll have a think about it. |
Its true. So why not adding a 'row' decorator? The default one would just render the default toggle and header. {row: [toggle, header]} Also you could think about handing the decorators one by one as props, e.g. renderToggle, renderHeader, renderRow, renderLoader. Your current approach only allows for all or nothing. |
+1 I was looking for this exact functionality! Otherwise parts of the tree get toggled when all the user wants to do is select a particular item. |
@bkniffler @gbmhunter I agree with you guys. I'm going to look into this issue in more depth, to find a good balance between extensibility and out-of-the-box usability. I'll take some ideas from @bkniffler's work and try to get a good mix. |
@bkniffler @gbmhunter. I have release |
Cool, couldn't be happier with your new version import {Treebeard, decorators} from 'react-treebeard';
import TreeStyle from './tree-style';
var Container = (props)=>{
// Handle node render with inline loading indicator, custom toggle and header
}
const Loading = (props) => <div></div>;
<Treebeard data={lists} decorators={{...decorators, Loading, Container}} onToggle={updateTreeItem} style={TreeStyle} animations={false}/> Separating the decorators as multiple props would still make this a bit better. But it works great! |
ps. Feel free to remove my pull requests as soon as you're done incorporating the parts you like. |
We also need to be able to expand/collapse the tree only by the toggle icon. |
Hey,
Sometimes its necessary, to only toggle if explicitly the trigger component was clicked.
Currently I'm overriding the Header and I put a click handler with preventDefault() on my node.
I think there should be a better way for handling that case. Maybe the onClick should be placed in the default decorator Header/Toggle component, and if overwritten, we handle it ourself.
The text was updated successfully, but these errors were encountered: