WIP - feat(RowDetail): add 2x new subscribe methods & fixed multiple issues

[ghiscoding]

DO NOT MERGE
This PR will be replaced by another PR that we are currently ironing out in my local repo.

This PR might also close #252

• add onRowOutOfVisibleRange subscribe
• add onRowBackToVisibleRange subscribe
• expose collapseItem public
• add new keyPrefix property to change the key prefix of all metadata used by the plugin that appears on the item object
• add new collapseAllOnSort boolean flag to provide possibility to implement it's own code for on sort
• add new saveDetailViewOnScroll boolean flag because saving detail is not used by all frontend frameworks
• use item everywhere instead of itemDetail in some areas (however make it to work with both so that it doesn't break anyone's code)
• changed all containers to classes instead of ids (e.g. id="detailViewContainer_2" is now class"detailViewContainer_" on the DOM element)
• fixed cssClass which was not fully implemented in the code
• add Filter example and padding tweak
• add JS code with dynamic template examples
• fixed some issues found and cleaned up unused code

There's a lot of changes, but I believe that it will be a simple plug & play for most users. The biggest feature is the additions of 2x new subscribe methods to let the user know when the Row Detail View is out or back to visible range. Also a working sample with Filters is a nice addition.

[ghiscoding]

@SatanEnglish
I would like you to test the new code before I merge the code. As mentioned on top, it should be a simple plug & play (or copy & replace) on your side. If you could just copy the JS file slick.rowdetailview.js and try it out in your environment, that would be great.

Also note that I updated the demo to show some JS examples of a button click in the dynamic template. I use the 2 new subscribe methods to trigger a refresh of the JS (basically re-attaching a button onClick)

[SatanEnglish]

@ghiscoding

I'll have a quick try now to update my code to use the new stuff and see how it goes.
I'll try get to you ASAP can only spare a couple of hours fiddling with this tho.

[ghiscoding]

@SatanEnglish
That's ok, just want to make sure that it works on your side, I would rather not break anything.

I also forgot to mention, that the function handleScroll you made was renamed to handleOnScrollSaveDetailView as you can see here and is not being called anymore. The reason is that I left it there is in case the new code is not saving as you wish. My new implementation is in calculateOutOfRangeViews() which also call saveDetailView() like you used to do in the handleScroll but in a more efficient way. So I would like to know if it works correctly on your side, if it does then I will delete your previous code, shown here... if it doesn't work properly, then try uncommenting the line here to save as before. Let me know what works.

Thanks

[SatanEnglish]

@ghiscoding

Couple of things I have in my current version that are missing from here.
I apparently hadn't updated git in a bit.

in resizeDetailView I now have.

        // replace
        //var itemHeight = inner.clientHeight;

        // Get the scroll height for the main container so we know the actual size of the view
        var itemHeight = mainContainer.scrollHeight;

        // Lastly save the updated state
        saveDetailView(item);

Think the last saveDetailViewis so that I can call resizeDetailView on outside and it resize and save the new size.

Also I have

• expandedClass: Extra classes to be added to the expanded Toggle
• collapsedClass: Extra classes to be added to the expanded Toggle

e.g in detailSelectionFromatter

            var expandedClasses = _options.cssClass + " collapse ";
            if (_options.expandedClass) expandedClasses += _options.expandedClass;
            html.push("<div class='" + expandedClasses + "'></div></div>");

[SatanEnglish]

@ghiscoding

I also resize my grid every now and then.
For example some pages buttons appear and therefore my grid height has to change.
This calls resizeCanvas and sets new limits.

Question here is will I need to calls your calculateVisibleRenderedCount to recaculate the height?

[SatanEnglish]

@ghiscoding
Like wise the collapsed version

            var collapsedClasses = _options.cssClass + " expand ";
            if (_options.collapsedClass) collapsedClasses += _options.collapsedClass;
            return "<div class='" + collapsedClasses + "'></div>";

[SatanEnglish]

@ghiscoding
detailSelectionFormatter I also had to add a max-height on it so scroll view would always be applied
when to big.
Least I think that what it was for.

e.g.

            html.push("<div class='detail-container detailViewContainer_", dataContext.id, "' style = 'max-height:" + (dataContext._height - rowHeight + 5) + "px'>");

Don't think I could handle this through pure css. (Think it's that the calculation for height is our by nearly a row and this is to deal with the little extra white space at the bottom)

[ghiscoding]

@SatanEnglish
You're talking about a few changes and you kind lost me in all these changes. So instead of trying to modify the changes here, I recreated the PR in my own repo. Can you please go in my copy of Slickgrid - RowDetail and do the changes directly on it. I made you a collaborator on it, so you can push code.

detailSelectionFormatter I also had to add a max-height on it so scroll view would always be applied
when to big.

I removed 1 of the max-height because the parent already had a max-height and on my side it was breaking so I had to do a max-height: none !important but then I saw it had no effect on the Row Detail Example.

Question here is will I need to calls your calculateVisibleRenderedCount to recaculate the height?

I added a trigger on browser resize to recalculate the visible lines because if you make the window bigger then the amount of displayed rows might change and it affects my calculation
$(window).on("resize", calculateVisibleRenderedCount);

calculateVisibleRenderedCount is also called by the onScroll, it has to calculate every time. So it's similar to your previous code but a bit more efficient since I don't ready the row object, I just do my calculation with the row index.

Couple of things I have in my current version that are missing from here.

You can look at the PR for all the changes made to the file itself. I did lot of cleanup, so yes it's possible that I deleted some of your previous code.

[SatanEnglish]

@ghiscoding

mainContainer.setAttribute("style", "max-height: " + item[_keyPrefix + 'height'] + "px");

cannot be used on a document.getElementsByClassName
This is Y this was an ID not a class definition orginally

[SatanEnglish]

@ghiscoding
In response to ur responce
Couple of things I have in my current version that are missing from here.
I did mention in there that these were items I hadn't got round to uploading it wasn't you had deleted them.

[SatanEnglish]

@ghiscoding
The one for mainContainer is really troubling is there a better way to be doing it?

I understand why you have put it as a getElementsByClassName instead of the getElementById.
e.g. so that it works with multiple grids.

BUT how do we make it so that it's unique to update attributes hence Y it was an id

[ghiscoding]

@SatanEnglish
Using classes is more for consistencies, SlickGrid uses classes everywhere, including row and cell positions (e.g. slick-cell l2 r2 cell-title for the 2nd row on the 2nd column. Also technically speaking, using class instead of Id doesn't make much difference since his name still includes the row Id in it. However, if it's really causing problem and we can't use classes then we can revert back to ID.

All that I can see is, it's changing from <div id="detailViewContainer_2" class="detail-container"> to div class="detail-container detailViewContainer_2">, you can still call $('.detailViewContainer_2') with jQuery to find exactly that single Row Detail and not any other... By the way, I don't know why it's vanilla javascript instead of jQuery there. Maybe that comes from copy+paste from the Violet's original code.

Again feel free to do some commits on my repo on the branch name feature/row-detail-view.

[SatanEnglish]

@ghiscoding
Issue is that it needs to change an attribute on and element.
$('.detailViewContainer_2') still gets multiple elements.

For example doing $('.detailViewContainer_2')[0] would be what the current code does.

Is there a unique ID on the grid I can get to? Then we could append that to the class name

[SatanEnglish]

@ghiscoding

// This has to be like this else it doesnt work
_grid.getOptions().minRowBuffer = _options.panelRows + 3;

e.g. With

Your version
_gridRowBuffer = _gridOptions.minRowBuffer;
options.minRowBuffer = _options.panelRows + 3;

Do you get the same issue ur end? Or have u not tryed with large view in there

[SatanEnglish]

@ghiscoding
Right I'll try have a play over the weekend think I've ironed out all of the missing issues.
I'll have a go at making some example to break your stuff so that we can both see and deal with fixing them.

I'll have a go at updating your PR when I do

[SatanEnglish]

@ghiscoding
Do you use discord?
I would be nice to start a channel on it so that I can talk through these issues with you.

So that I can get some idea around the best way to fix some stuff

[ghiscoding]

@SatanEnglish
Let's go and continue the conversation in a new issue on my repo here

[ghiscoding]

closing, replaced by #287