Explore usage in react-big-calendar

[jquense]

👋 This isn't a complete list at all but I have a moment so thought I'd mention some initial impressions/concerns i'd have in the context of the TimeGrid.

1. Each Selectable node in the context of a container is measured (get bounds) on each instance of mousemove, in the case of a Month view, at most 42 nodes are being measured per hit so its not much of a concern. In the TimeGrid, tho there is a lot more segments being measured, and at the frequency at which they are measured i'd be concerned for performance (which existed in our early attempt at this)
2. It doesn't really suit the use case of overlaying something over the selected region...you can make it work by exposing the DOM nodes and introspecting their bounds and sorting them correctly...but I think that starts to really conflate concepts...I'd like to not have to think about the DOM nodes at all for components that I don't own directly. To some extent our use case does conflate the idea of a date slot with a DOM node, but that's a actually incidental, a user isn't actually selecting the component outlining a 30min chunk they are drawing and event over them.
3. I may have missed that it does this, but selection needs to be cancelable. in rbc a user can prevent a selection over a specific range which is useful to limiting the size of an event

I realize that our two use-cases can be coerced into the same one, but I think that's not because they are really the same underneath, but because they are close and you can sort of fake it. It'd be nice if this lib could manage both...I don't actually mind doing the slot calculation stuff inside of the DaySlot, but it definitely would be nice to be able to decouple that logic from the component itself via a HoC or similar.

[cellog]

hey thanks for looking into this! Quick responses:

1) In the TimeGrid, each day is handled separately, so we are looking at a default of 24 separate slots per click. However, I do want to clarify one design element. Since the actual handling of selection might change the CSS (for instance, a selected element changes size), I wrote it this way. HOWEVER, for use cases like big-calendar, I plan to add an option to cache the bounds of each element, so that it is done once, at registration time. Then, by passing a prop to the selection, you can enable or disable caching on the fly. Part of the reason I moved the mousedown handler to the Selection HOC's onMouseDown was to be sure that only the relevant container's internal selectable things were checked.

1. Perhaps I'm reading the code wrong, but I thought react-big-calendar does the same thing, which is to say the code figures out what slots are overlapped. However, to avoid touching the DOM, it makes some assumptions (all slots are the same height and width). I agree that the user is drawing an event, but I have always thought I was selecting time in google calendar :). Then it decides what to do with the time I have selected, usually by making an event popup.
2. I have a couple of plans to implement this exact thing. My main plan was to add type-based selection, much like React DnD. So you would register selectable things as being of a certain type. The Selection container can then dynamically be told (through props) what kind of types it can select. This way, a selection stops at a boundary of a differently typed thing. The Selection container would decide what kind of element types to select based on whichever was under the mousedown, if more than 1 type can be selected. Otherwise, it will select only the things that are registered that have a particular type. I'd love to see an example of how rbc makes selection cancelable, it would clarify how to implement this.
3. calculation inside DaySlot: Actually, I was kind of hoping that this code (the "makeFancy" and even the creation of the selection div itself) would be entirely outside of DaySlot. Because this lib passes the actual DOM elements that were selected, we can easily write the code to display the selection block anywhere, and either position it absolutely or whatever we need to do to get it in the right place on the screen. I only put it in DaySlot so it would be similar to the old code.

So, to summarize, the primary goals I had with the selection code are:

1) allow abstractly selecting values without needing to know anything about how they are represented or laid out on the screen 2) allow formatting stuff within each selected thing without needing to know anything about other things around it that are selected (the Selectable HOC passes in a boolean "selected" prop) 3) allow formatting unifying visual abstraction of the selection, by having the actual selected DOM elements, so we can query their geometry -> this last part is used for making the selection box in TimeGrid.

[cellog]

Can I also take a stab at summarizing the blocking issues for using in rbc:

1) performance needs to be as good or better 2) selection needs to be cancelable/stoppable at a boundary. I'd also like to define some way of grouping selectable elements, so if a selection starts inside one group, it won't select any other groups. Two ways to do this: define adjacency, and define a simple grouping callback based on value. Neither of those seem very elegant to me, so I need to think further. 3) questions about the logic itself. Re-reading what you wrote, I could just as easily take the values that were selected, calculate the slot and css needed, and ignore the dom elements passed in to the callback, which would satisfy what you're talking about.]

Once I have specific things I can try to fix, either you or I can open up new issues for each one so that I can track them

[jquense]

selection needs to be cancelable/stoppable at a boundary.

It's not just at a boundary, the user can prevent selection for any reason at any moment during selection: https://github.com/intljusticemission/react-big-calendar/blob/master/src/DayColumn.jsx#L214

I'd certainly prefer a generic way to cancel/halt selection vs builtin types or adjacencies which may only fit narrower use-cases and are could easily be thwarted by the idiosyncrasies of css layout. The use-cases that prevent themselves for that is things where you need to drag "inside" a selection group but not select anything (drag and drop events for instance), or just wanting to disable selection while the mouse is clicking somewhere, etc. I think it''d be too much to try and anticipate all use cases to handle.

3) allow formatting unifying visual abstraction of the selection, by having the actual selected DOM elements, so we can query their geometry -> this last part is used for making the selection box in TimeGrid.

This is the bit that feels leaky to me. I totally understand why you'd do that, but I think that there are actually a lot of implicit presumptions in tying the nodes to a value, there biggest being that layout of the nodes on the page may not be related to value sort order. It's straightforward in the TimeColumn example because they are a stack. but you'd not be able to make those assumptions about absolutely positioned things. Similarly with the sorting, that feels like an API that will never quite meet everyone's use cases. I think it'd probably be preferable to not have to ask for it. It seems good to provide the nodes as an escape hatch but i'd generally prefer that the code not make assumptions about layout of said nodes (to the extent possible of course)

calculation inside DaySlot: Actually, I was kind of hoping that this code (the "makeFancy" and even the creation of the selection div itself) would be entirely outside of DaySlot.

I don't have a strong preference for it being there, but I think pulling it out may pose a challenge of extra divs floating around which makes flex-box unhappy.

[cellog]

so in case you're wondering why I've fallen off the map, I've been mud-wrestling with saucelabs to get it set up. Finally got it working, then found a VERY subtle bug in pageOffset (which would affect react-big-calendar too, fyi). In iOS, it just plain doesn't work if the code is running in a frame. So, one has to check to see if the parent window's scroll offsets are larger than the current window, and use those instead if so.

Anyways, I'm working my way through the other assumptions (like getElementFromPoint, and getBoundingClientRect) just to be sure it's all correct. When it is, I'll move on to the testing of events and such, make sure it all works in every browser.

Then I'll get back to you on your questions, all of which I am sure are solvable, you'll be happy to hear.

[cellog]

A quick note about your final concern: the Selection HOC expects 2 sorters to be passed in. The first sorts the nodes in the order that makes sense logically, the second sorts the values. So if the logical node order is what is visual, you simply use () => 0 as the sorter, and it will return everything in the order it arrives in (DOM order). In other words, there is no assumption about order for the nodes. You could sort them randomly, and that would be the order that is returned to the user.

I finally finished my wrestling match with karma and saucelabs, got them working, so I set about step 2 of the refactor, and split off the input handling and the selection handling from the HOC. Now each portion of the thing is easily unit testable, and strongly decoupled, so I can make changes in one area without seeing weird behavior from another. It also reduced code size compared to the original and is a lot easier to understand.

[jquense]

the Selection HOC expects 2 sorters to be passed in.

Ya I realize you can noop them, my point was more it'd be better if it didn't care at all and didn't even ask for a sorter. just requiring that I sort them is making an assumption: that they are sortable. Why sort them at all?

[cellog]

react-big-calendar implicitly does. By finding the first and last slot, it then sorts the selected nodes in slot order. If you drag down, you get them in date order, but if you drag up, you get them in reverse date order. Since the selection library is unaware of what order is, unless you really don't care how they are sorted, you'll want to sort them. There are a couple of ways of doing this. The code I wrote only sorts when the mouse is released/finger is released, unless you pass an onSelectSlot callback, in which case it sorts them as it goes. Alternately, if it adds a new element and you have constantSelect enabled, it calls the onFinishSelect callback every time a new one is added. So, it's not a big performance penalty, unless you are a robot pretending to be a person selecting.

I just added a story with 1000 nodes in it, to see how fast it selects/unselects, and will use that to optimize when I've got tests.

Basically, the sorting could be made optional, but I'd like to see proof that it is a significant performance drain first. And I will find that evidence, you don't need to worry about it.

[jquense]

react-big-calendar implicitly does.

Right, I think it makes sense for the consumer of the selection logic to sort them, I don't think it makes sense for the selection library to do it...I'm not concerned about performance on this bit, this is more a general API thought. To the extent that you don't need to have opinions or deal with the content of the data it seems like a good idea not too. Also I more was talking about this bit: https://github.com/cellog/react-selection/blob/master/src/Selection.jsx#L68

[cellog]

right, that is the part that returns the data in the order that you define it to be in. It's simpler for the library to sort the nodes for you than it is for you to do it. Again, I can make it disableable, but I don't see why this is such a bad thing? It's 2 lines of code and an unnoticeable hit on my machines and in my testing on BrowserStack on other real devices

[jquense]

If you like it go for it. I just think its a weird thing for the library to do at all, if its not required to calculate selection. No right or wrong it just feels like a bonus opinion the library has :)

[cellog]

Basically, it's a DRY thing. Most users will have to write this, and if they use it in multiple places (for instance, for a selectable table with selectable columns, etc.) then it results in lots of rewriting what I could do for them. But I haven't written the example selectable table yet, so we will see. What will be interesting is writing code where a selection container is itself a selectable thing if you drag from a parent container that is also selectable. Oodles of fun awaits :)

[cellog]

ok, so did a test on the 1000 node thingy. Caching bounds of nodes results in a small performance improvement. However, the huge performance hit comes from React. Fully 52% of the time spent churning is occupied by React, although I'm not sure if that would happen in production mode. But the sorting code doesn't even show up in either case. I'm also pleased to see the objectsCollide code takes almost no time

[cellog]

hey just had a brainstorm: when doing selection, if the selection rectangle is south to north, I'll just insert newly selected items at the front of the selection array. Voila, instant sorting by DOM order. The sorters will only be used if supplied. This should address both your concern and mine.

[cellog]

bad news: not sorting doesn't work. Here's an example scenario:

We have a grid of selectable items, laid out 5 on a row. The user starts by selecting number 3, then drags down to number 8. Now we have [3, 8] selected. Then the user drags left to number 7, so we have [2, 3, 7, 8] selected. Since we were dragging down and to the left, with the naive approach (push if dragging down, unshift if dragging up), we get [3, 8, 2, 7]. So the only way to guarantee correct sorting order is to sort it at some point. Because I have an operation to insert each selected value anyways, I'm going to use insertion sort, instead of an end-of-select sort() call. It should be pretty efficient, I'll start the sort at opposite ends depending on the value I'm inserting and the value at the edge of the array. This crap is complicated. No wonder algorithms classes focus on sorting so much.

[jquense]

I'm still not sure what sorting is for...can't you calculate whats selected based on whether the selection rectangle collides with the node, who cares what order they are in?

[cellog]

No. For an example, let's say we want to select dates in a month. Our mouse selects a rectangle that contains the 2nd and the 3rd, 9th and 10th. The user expects us to select 2, 3, 4, 5, 6, 7, 8, 9, and 10. The values between the extremes are implied. In this case, we can just fill in the values from the highest and lowest, which requires Math.min/Math.max or an iteration equivalent. But this means we re-generate the list of values and their nodes, which is the same as sorting them (we have to go over the list twice, once to determine the selected nodes and their boundaries, a second time to generate the sorted list)

In other words, it is a very common use case to imply selection of items that don't fall within the selection rectangle. Without sorting, we can't do this properly, whether it is by implied sorting (fill in the min/max) or actual sorting.

[jquense]

that's right forgot about that one... you can see the complexity of the problem. There are also plenty of cases where you would not intermediate values selected as well. The complexity of use cases is part of the reason the RBC code is handled at a specific, fairly non general way at a high level. :P

[cellog]

it's not bad, though. Just had to realize I need to abstract out a layer for the actual tracking of selected items, and it's stripping away oodles of complex code. You'll see in the next commit 🎱

[cellog]

FYI, the non-general high level way selection is handled in RBC means to add slightly different selection requires much more coding than I want to do inside RBC. What I'm working on now is something where I can toggle a switch in the props to turn different selection modes on or off, without twiddling with the rendering of the calendar, which is frankly irrelevant to how I select parts of it or not. That's what I hope to convince you of, once I've got this thing rassled to the ground.

[cellog]

OK. So I figured out the basic problem I was not getting. Selection is a 3 step process.

Step 1: get the ordered set of things that are already selected (empty for a new selection) Step 2: get the ordered set of things represented by the selection rectangle of start to finish click/tap-drag and any extras (in-between values) Step 3: use set math to figure out what is selected and not selected. So for additive, we use xor. For regular selection, we use or

Voila! Fully working and much simpler to maintain.

Working on selection cancellation right now. What I'm imagining is that the onSelectSlot callback (now named onSelectionChange) has a 3rd parameter, a function you can call like this:

onSelectChange(selectThingy, bounds, cancel) {
  if (selectThingy.selectedIndices.length > 4) {
    const min = 3
    const max = selectThingy.selectedIndices.length - 5
    cancel({ indices: Array.apply(min, Array(max - min)).map((x, y) => min + y + 1 })
  }
}

This example would limit the selection to 4 items from the first one, no matter what. Also possible would be passing in an array of values or array of nodes.

Any other way of canceling selection you'd want?

[jquense]

hmm in not sure about that cancelation api...it is fairly unidiomatic and seems overly complex. Why not just return false from the event? I'd have to expose this to RBC users, and having them call cancel function with a magic object is a bit odd. It seems like selection cancelation should purely be a matter decided based on the values that would be/are selected. All the leaking of DOM nodes, and ordered lists of things exposes a user to way more implementation details than i care for.

[cellog]

ok, I can make it cancel the whole selection by returning false. The finer grained stuff is available for cancelling part of the selection

[cellog]

fyi, this would be callback exposed to rbc. I am imagining you'd expose different stuff to users, rather than just exposing it directly

[jquense]

O sorry I didn't mean the whole selection, just the current one. So RBC does this now, as you add/remove a new slot from the current selection you get a callback, if you return false the last change to the selection is disregarded, So you as you drag the user gets all the current slots (values) and if say the the distance between the start and end is > 4hrs they cancel, effectively limiting the selection to 4hr segemnts.

[cellog]

ok. Does this work in month view also? So let's imagine the user starts on a monday and drags down to the following monday. That would select monday first. So on the 2nd request, the user gets tue,wed,thu,fri,sat,sun,mon as the new selected items. Cancelling removes tuesday->monday. Is that the desired behavior?

[cellog]

All right. #1 and #3 of your initial concerns are fully addressed. Where do we stand? About to release 0.4.0. Have to document it next on the website

[jquense]

Not sure, I don't honestly have a lot of time in the near future to try it out (busy at work!). I also gotta figure out how we feel about the size increase, It's probably not a big deal but this library is significantly larger then the code we have ;) I do realize that comes with extra features :)

[cellog]

I expect that it will be about equal size once the existing code that has to be repeated (all the life cycle methods and such) is removed. Also, I still think there is lots of room for size optimization in the react-selection-hoc :)

I will do a PR once things are out, will probably be 2-3 weeks.

via CloudMagic Email [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=8.5.43&pv=5.1.1&source=email_footer_2]

[jquense]

I expect that it will be about equal size once the existing code that has to be repeated

Not sure I understand that one...we can pull out the existing code into a HOC without much effort. just looking at the code right now in RBC including setup/teardown it's probably 1/3 the code as the library here.

I'm not saying thats necessarily a bad thing, but there is no way around the fact that this library is much bigger then the current solution

[cellog]

No need to debate. PR will make it clear

via CloudMagic Email [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=8.5.43&pv=5.1.1&source=email_footer_2]