Remove disable editor #343
Conversation
| @@ -140,7 +140,8 @@ const EventItem = ({ event, multiday, hasPrev, hasNext, showdate = true }: Event | |||
| onEventClick(event); | |||
| } | |||
| }} | |||
| disabled={event.disabled} | |||
| focusRipple | |||
| disabled={disableViewer || event.disabled} | |||
There was a problem hiding this comment.
This change could lead to a regression to a fix addressed here: #338
The purpose of that PR was giving the user the ability to listen for event click without using the viewer (e.g. custom navigation)
There was a problem hiding this comment.
got it
can you propose a fix that an event should not be focusable if its not clickable?
There was a problem hiding this comment.
In my opinion also with disableViewer=true events should be clickable (and focusable). With this property someone can use the calendar and intercept event click to attach custom logics, without opening the popover provided by this library.
There was a problem hiding this comment.
Pay attention also on this line:
The disabling logic should remain aligned between this two components.
| @@ -71,7 +71,6 @@ All props are _optional_ | |||
| | viewerExtraComponent | Function(fields, event) OR Component. Additional component in event viewer popper | | |||
| | viewerTitleComponent | Function(event). Helper function to render custom title in event popper | | |||
| | disableViewer | boolean. If true, the viewer popover will be disabled globally | | |||
| | disableEditor | boolean. If true, the editor modal will be disabled globally | | |||
There was a problem hiding this comment.
I understand that the use of editable could prevent the opening of the editor, at the same time my intention was to provide a way to click a grid cell, intercept the event and attach a custom logic.
Unfortunately with editable=false the recently introduced onCellClick is not called any more.
At the moment the only way I have to intercept the click is to implement a custom editor and close it programmatically (something similar of this hack on viewer side: #329 (comment)) and having the screen flashing for the backdrop appearing and disappearing immediatly.
There was a problem hiding this comment.
it seems to me that making it focusable without any click result (the popover) is unusual, it should not be the case.
I think you can resolve this using tabIndex of -1 if its disabled
can you open a PR for that?
There was a problem hiding this comment.
Before doing any new PR I'd like to explain my needs so we can find the more appropriate way to implement ;)
In some scenarios I need to use the calendar disabling completely the default modal / popover. If a user click an event I only need a callback with event details. If a user click a cell on the grid I only need the cell details (start, end, resource...). I need also that in this case the grid works as usual (click, focus, and so on).
disableViewer was a good way to do it but no events were raised, because of the button disabled so in PR #338 I addressed a fix but now in this PR the fix was reverted.
I think buttons should not be disabled when disableViewer=true because we are asking the component to disable the popover, not events.
The same IMHO should be done for the editor: I need a way to disable completely the popover and give the possibility to the developer to implement its own logic.
I'll give you an example of why I am asking this: I need to fill some dates in a component of my application and such dates come from the interaction with events or the grid.
There was a problem hiding this comment.
I'm trying to make things generic and not for specific needs
IMO disabled button should not be focusable
To go around this and fulfill your needs, we can apply tabIndex={-1} without disable. In this case, the button will not be disabled, and also not focusable.
Does this sounds good for you?
There was a problem hiding this comment.
Let's try and do some experiments before merging so we can discuss on it. Can you open the PR (I'm not sure it's 100% clear for me)?
There was a problem hiding this comment.
Ok, regression on clicks has disappeared.
I have another issue: with editable={false} I also lose the ability to drag & drop, is there a way to restore it?
BTW for the moment we can merge
There was a problem hiding this comment.
I have another issue: with editable={false} I also lose the ability to drag & drop, is there a way to restore it?
have a look #346
There was a problem hiding this comment.
Ok, regression on clicks has disappeared.
I have another issue: with editable={false} I also lose the ability to drag & drop, is there a way to restore it?
BTW for the moment we can merge
This is intended. drag & drop is an edit action. if editable is false, then it should not be dragged
disableEditorand makeeditableaffectcellsas well aseventsdisableVieweristrueadjustment for #342