Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Row detail view + row draggable creates weird behavior #256

Closed
hramli opened this issue Aug 1, 2019 · 9 comments
Closed

Row detail view + row draggable creates weird behavior #256

hramli opened this issue Aug 1, 2019 · 9 comments
Labels

Comments

@hramli
Copy link

hramli commented Aug 1, 2019

IMGUR LINK TO DEMO: https://imgur.com/a/lUTxqKV

Your Environment

Software Version(s)
Angular 8.0.3
Angular-Slickgrid 2.9.9

Context

Trying to create a grid with draggable rows for rearranging + rows with detailed view.

Expected Behavior

When rows are expanded to detailed view and dragged to be rearranged, detailed view should still be the same and contents of neighboring rows should not be affected.

Current Behavior

When rows are expanded and dragged, the detailed view disappeared. Also, contents of the rows between the original row position and the new row position disappeared / are missing.
(can be shown in imgur: https://imgur.com/a/lUTxqKV)

Possible Solution

Code Sample

//custom formatter
const statusFormatter: Formatter = (row, cell, value, columnDef, dataContext) => {
  return value ? '<i class="fa fa-circle" style="color: green;" ></i>' : '<i class="fa fa-circle" style="color: red;"></i>';
}

//local storage key for grid state
const LOCAL_STORAGE_KEY = "gridState";

@Component({
  selector: 'app-grid',
  templateUrl: './grid.component.html',
  styleUrls: ['./grid.component.css']
})
export class GridComponent implements OnInit, OnDestroy {

  constructor(private translate: TranslateService,
              private dataSvc: DataService){
    
  }

  columnDefinitions: Column[] = [];
  gridOptions: GridOption = {};
  dataset: any[] = [];
  angularGrid: AngularGridInstance; 

  originalState: any;
  num: number = 0;

  gridSubscription: Subscription;

  ngOnInit(): void {
    const presets = JSON.parse(localStorage[LOCAL_STORAGE_KEY] || null);
    this.defineGrid(presets);
  }

  ngOnDestroy(){
    if(this.gridSubscription)
      this.gridSubscription.unsubscribe();
  }

  resetPreset(e: any){
    localStorage.setItem(LOCAL_STORAGE_KEY, null);
    // this.defineGrid(null);   => weird behavior
    this.angularGrid.gridService.resetGrid(this.columnDefinitions); //correct 
  }

  defineGrid(gridStatePresets?: GridState){
    this.columnDefinitions = [
      { id: '#', field: '', name:'', width: 40, behavior: 'selectAndMove', selectable: false, resizable: true, cssClass: 'cell-reorder dnd'},
      { id: 'status', name: 'Status', field: 'status', formatter: statusFormatter},
      { id: 'date', name: 'Date', field: 'date', sortable: true}
    ];
    this.gridOptions = {
      enableAutoResize: true,       // true by default
      enableCellNavigation: true,
      enableRowMoveManager: true,
      rowMoveManager: {
        onBeforeMoveRows: (e, args) => this.onBeforeMoveRows(e, args),
        onMoveRows: (e, args) => this.onMoveRows(e, args),
        cancelEditOnDrag: true
      },
      enableRowDetailView: true,
      rowDetailView: {
        process: (item) => this.setSelectedAirline(item),
        loadOnce: false,
        singleRowExpand: false,
        useRowClick: true,
        preloadComponent: SpinnerComponent,
        viewComponent: RowDetailViewComponent,
        panelRows: 3
        // expandableOverride: () => {return false}
      }
    };

    this.gridSubscription = this.dataSvc.apiData.subscribe(
      data => {
        this.dataset = data;
      }
    );

    if(gridStatePresets) 
      this.gridOptions.presets = gridStatePresets;
  }


  angularGridReady(angularGrid: AngularGridInstance){
    this.angularGrid = angularGrid;
  }

  saveGridState(grid) {
    //parses object into string to store in local storage
    localStorage.setItem(LOCAL_STORAGE_KEY, JSON.stringify(this.angularGrid.gridStateService.getCurrentGridState()));
  }

  onBeforeMoveRows(e, data){
    for (let i = 0; i < data.rows.length; i++) {
      // no point in moving before or after itself
      if (data.rows[i] === data.insertBefore || data.rows[i] === data.insertBefore - 1) {
        e.stopPropagation();
        return false;
      }
    }
    return true;
  }
  
  setSelectedAirline(item: airlineStatus){
    this.dataSvc.selectedData.next(item);
    return this.dataSvc.selectedData;
  }

  onMoveRows(e, args) {
    const extractedRows = [];
    let left;
    let right;
    const rows = args.rows;
    const insertBefore = args.insertBefore;
    left = this.dataset.slice(0, insertBefore);
    right = this.dataset.slice(insertBefore, this.dataset.length);
    rows.sort((a, b) => {
      return a - b;
    });

    for (let i = 0; i < rows.length; i++) {
      extractedRows.push(this.dataset[rows[i]]);
    }

    rows.reverse();

    for (let i = 0; i < rows.length; i++) {
      const row = rows[i];
      if (row < insertBefore) {
        left.splice(row, 1);
      } else {
        right.splice(row - insertBefore, 1);
      }
    }
    this.dataset = left.concat(extractedRows.concat(right));
    const selectedRows = [];

    for (let i = 0; i < rows.length; i++) {
      selectedRows.push(left.length + i);
    }

    this.angularGrid.slickGrid.resetActiveCell();
    this.angularGrid.slickGrid.setData(this.dataset);
    this.angularGrid.slickGrid.setSelectedRows(selectedRows);
    this.angularGrid.slickGrid.render();
  }
}
@ghiscoding
Copy link
Owner

ghiscoding commented Aug 1, 2019

oh wow it's incredible to see what users can do with 2 features that were never tested together lol. What I mean is that these 2 features were designed and created independently in the SlickGrid core library, and we never tested dragging and row detail (the later is fairly new, just a few months old) and there's probably still some bugs like yours that may arise when combined with other plugins (row detail & draggable row are 2 separate plugins within the core lib).

Anyhow I think that we might need to subscribe internally to the draggable event and close all row details when that happens. There's way too much logic to put in when dealing with row detail and moving rows. For example, we do exactly that when doing a sort, you'll see that when you sort it closes all row details.

You might be able to apply a quick fix to collapse all row detail before moving the rows. Try the following in your code

onBeforeMoveRows(e, data) {
   // collapse all row detail panels before moving any rows
   const rowDetailInstance = this.angularGrid.extensionService.getSlickgridAddonInstance(ExtensionName.rowDetailView);
   collapseAll();

   // the rest of your code
   // ...
}

Since these 2 plugins are rarely used together, I think that would be the only option and I don't think I should add that in the lib itself because the onBeforeMoveRows event is only available when using the rowMoveManager option. In other words, it has to be fixed in your code (with what I provided, hopefully that works), please confirm that it works (I didn't have time to create a Wiki for the Row Detail, but that info should go in a Wiki).

@hramli
Copy link
Author

hramli commented Aug 1, 2019

Thanks for the quick response.
This works for me:

onBeforeMoveRows(e, data){
    const rowDetailInstance = this.angularGrid.extensionService.getSlickgridAddonInstance(ExtensionName.rowDetailView);
    rowDetailInstance.collapseAll();

    for (let i = 0; i < data.rows.length; i++) {
      // no point in moving before or after itself
      if (data.rows[i] === data.insertBefore || data.rows[i] === data.insertBefore - 1) {
        e.stopPropagation();
        return false;
      }
    }
    return true;
  }

Also, is rowDetailInstance a RowDetailView object?
I get this when I logged rowDetailInstance:

RowDetailView {init: ƒ, destroy: ƒ, pluginName: "RowDetailView", collapseAll: ƒ, collapseDetailView: ƒ,}
collapseAll: ƒ collapseAll()
collapseDetailView: ƒ collapseDetailView(item, isMultipleCollapsing)
destroy: ƒ destroy()
expandDetailView: ƒ expandDetailView(item)
expandableOverride: ƒ expandableOverride(overrideFn)
getColumnDefinition: ƒ getColumnDefinition()
getExpandedRows: ƒ getExpandedRows()
getFilterItem: ƒ getFilterItem(item)
getOptions: ƒ getOptions()
init: ƒ init(grid)
onAfterRowDetailToggle: Event {subscribe: ƒ, unsubscribe: ƒ, notify: ƒ}
onAsyncEndUpdate: Event {subscribe: ƒ, unsubscribe: ƒ, notify: ƒ}
onAsyncResponse: Event {subscribe: ƒ, unsubscribe: ƒ, notify: ƒ}
onBeforeRowDetailToggle: Event {subscribe: ƒ, unsubscribe: ƒ, notify: ƒ}
onRowBackToViewportRange: Event {subscribe: ƒ, unsubscribe: ƒ, notify: ƒ}
onRowOutOfViewportRange: Event {subscribe: ƒ, unsubscribe: ƒ, notify: ƒ}
pluginName: "RowDetailView"
resizeDetailView: ƒ resizeDetailView(item)
saveDetailView: ƒ saveDetailView(item)
setOptions: ƒ setOptions(options)
__proto__: Object

but when setting the type of rowDetailInstance like

const rowDetailInstance: RowDetailView = this.angularGrid.extensionService.getSlickgridAddonInstance(ExtensionName.rowDetailView);

I get the message that collapseAll() does not exist. The closest function is collapseAllOnSort()

Thanks

@ghiscoding
Copy link
Owner

ghiscoding commented Aug 1, 2019

The method getSlickgridAddonInstance gives you the instance of the SlickGrid plugin, these plugins in SlickGrid are opt in, they are not part of the core library itself. Basically if you want this feature (for example Row Detail), you need to add the SlickGrid plugin to your code. To make it easier, what I did in Angular-Slickgrid is that the user simply need to enable a flag (enableRowDetail in your case) and the lib will take care of importing and loading the correct SlickGrid plugin. You can see a list of all the SlickGrid Plugins, again like I said they are opt in but I made them all available in Angular-Slickgrid with simple flags to turn on/off.

Now that you understand what SlickGrid plugins are, there are also Controls, which are very similar to Plugins and are opt in as well. You can see the SlickGrid Controls (basically, ColumnPicker, GridMenu are the main ones).

So with all of that, I decided to use the term Extensions in my lib to cover the entire set of Controls/Plugins (here's the list of Extensions in Angular-Slickgrid), each of these Controls/Plugins are actual Extension in my code and are totally separate, which I found much better for debugging purposes (my first implementation was all 1 big Service and that was messy, now they are all separate).

Then finally, these Controls/Plugins are called in the core lib by newing them (for example new Slick.Plugins.RowDetailView({ options }), the getSlickgridAddonInstance will return you the instance of that new plugin call. Since we need to close all row detail, we need to call the plugin's method, which is why you need to call the getSlickgridAddonInstance and from there you can then call the collapseAll.

I should probably put that in a Wiki somewhere 🤣
Does that make any sense?
Feel free to ask, as I do would like to copy + paste this in a Wiki

Cheers ⭐️

@hramli
Copy link
Author

hramli commented Aug 1, 2019

Make sense to me 😄

Another question:
Based on my understanding, setting gridOptions.presets only works/affects the grid when we set it before the grid is initialized ( eg. setting it in ngOnInit and not (onAngularGridCreated) ) ?

I am trying to create a toggle to display/hide a specific column and so far nothing works for me.
Redefining columnDefinitions: Column[] and gridOptions: GridOption on button click does not work. Changing the presets gridOptions.presets also does not work.

Any thoughts or solutions?

Thanks 👍

@ghiscoding
Copy link
Owner

presets as the name suggest is to preset some filters on page load, never after. To do it afterward would require a lot of changes in the lib code to handle that.

I am trying to create a toggle to display/hide a specific column and so far nothing works for me.

What do you mean by toggle? Usually the word toggle means hide/show a column and that would be through the Column Picker (right + click on the header titles) or the Grid Menu. To do that later in the game (outside of the page load, meaning without presets), you could use the same technique that I use in the grid for the presets, which is to use the setColumns() method from the SlickGrid core lib Grid object. You can get the Grid object with angularGridReady and from that call grid.setColumns( /**all visible columns*/ ). You can see that I do exactly that with the presets of the columns in the Angular-Slickgrid component on this line, whatever is outside of this setColumns is hidden (the column shows up in the pickers ColumnPicker/GridMenu, but is unselected)... oh wait that is no longer working with new core lib version, dang a new bug 🤦‍♂

If you instead mean to hide the column completely from the grid (and the pickers all together),

@hramli
Copy link
Author

hramli commented Aug 2, 2019

So setColumns() works for the columns I defined in columnDefinitions.

However, the columns for dragging rows (enableRowMoveManager and RowMoveManager) and expanding the row (RowDetailView) which is normally added to the grid via gridOptions.enableRowDetailView and gridOptions.rowDetailView is not showing.
The column for the drag and drop which by default has the hamburger icon is also not showing.

I tried this and it does not work :

defineGrid(gridStatePresets?: GridState){
    this.columnDefinitions = [
      { id: '_detail_selector', field: '', name: ''},
      { id: '#', field: '', name:'', width: 40, behavior: 'selectAndMove', selectable: false, resizable: true, cssClass: 'cell-reorder dnd'},
      { id: 'status', name: 'Status', field: 'status', formatter: statusFormatter},
      { id: 'airlineCode', name: 'Task', field: 'airlineCode', sortable: true},
      { id: 'process', name: 'Process', field: 'process', sortable: true},
      { id: 'date', name: 'Date', field: 'date', sortable: true}
    ];

    this.gridOptions = {
      enableAutoResize: true,       // true by default
      enableCellNavigation: true,
      enableRowMoveManager: true,
      rowMoveManager: {
        onBeforeMoveRows: (e, args) => this.onBeforeMoveRows(e, args),
        onMoveRows: (e, args) => this.onMoveRows(e, args),
        cancelEditOnDrag: true
      },
      enableRowDetailView: true,
      rowDetailView: {
        process: (item) => this.setSelectedAirline(item),
        loadOnce: false,
        singleRowExpand: false,
        useRowClick: true,
        preloadComponent: SpinnerComponent,
        viewComponent: RowDetailViewComponent,
        panelRows: 9
        // expandableOverride: () => {return false}
      },
      //preset to hide hamburger icon (so that users cannot rearrange rows until they click the edit button)
      presets: {
        columns: [
          {columnId: '_detail_selector'},
          {columnId: 'status'},
          {columnId: 'airlineCode'},
          {columnId: 'process'},
          {columnId: 'date'}
        ]
      }
    };
// rest of code
}

//toggle view
toggleView(){
    //enable edit
    if(this.edit)
    {
      this.grid.setColumns([
        { id: '#', field: '', name: ''},
        { id: 'status', name: 'Status', field: 'status', formatter: statusFormatter},
        { id: 'airlineCode', name: 'Task', field: 'airlineCode', sortable: true},
        { id: 'process', name: 'Process', field: 'process', sortable: true},
        { id: 'date', name: 'Date', field: 'date', sortable: true}
      ]);
      console.log(this.gridOptions);
    }
    //turn off edit mode (default)
    else 
    {
      //show detail selector column to be able to expand rows
      //also hide hamburger column so users cannot rearrange rows
      this.grid.setColumns([
        { id: '_detail_selector', field: '', name: ''},
        { id: 'status', name: 'Status', field: 'status', formatter: statusFormatter},
        { id: 'airlineCode', name: 'Task', field: 'airlineCode', sortable: true},
        { id: 'process', name: 'Process', field: 'process', sortable: true},
        { id: 'date', name: 'Date', field: 'date', sortable: true}
      ]);
    }
}

GIF:
slickgrid_weird1

Any ideas?

Thanks ⭐️

@ghiscoding
Copy link
Owner

In my previous post, I mentioned that there seems to be a regression bug with setColumns(), it doesn't hide columns anymore and the Example 16 - Grid State & Presets using Local Storage is not working as expected. Typically the setColumns() will help in keeping the state of the columns, that is: 1- column visibility, 2- column width, 3- column position in the grid. It seems that number 1 is no longer working and 2-3 are no longer working in my current code (but is working in previous version). So I have to investigate that before anything else.

For the rest of the question, it seems that you are fighting against the lib, both the Row Move Manager and the Row Detail plugins uses the first column position (by using column definitions unshift to add their action icon. The position of this icon to be as the first column is important and I'm almost certain that each plugin expect that icon to be exactly at the first column, because the plugin need to watch the icon clicked, and then if you move the icon to be on 2nd or 3rd column position, the plugin might not work anymore.

Lastly, Angular-Slickgrid keeps an internal copy of the columns, for example all visible columns and their positions, if you come from the outside and use setColumns(), you are fighting against the lib.

So all in all, the code you are trying to add is prone to errors. There are certainly plugins that I don't think will work well together, like the row move and the row detail. You might find a way to get it working, and that would be great, but I just don't have the time to help you with this. I do 95% of the job alone in this lib (5% being small contributions and bug filling), I added a bunch of unit tests, now starting to add more Cypress E2E tests and I also contribute a lot to the core lib SlickGrid. So I just don't have the time to help with such non-conventional use case.

@ghiscoding
Copy link
Owner

Side note to add to previous post, if you have the time to investigate and make it to work in the Angular-Slickgrid, I would be more than happy to review any PR (Pull Request) to support that. For that, the best would be to fork the lib and add a new example for your use case, then troubleshoot it, get it to work and make a PR.

@hramli
Copy link
Author

hramli commented Aug 2, 2019

Gotcha.
Thanks 👍

@hramli hramli closed this as completed Aug 2, 2019
ghiscoding pushed a commit that referenced this issue Apr 7, 2020
- now create column definition just by the enableRowMoveManager flag
- also keep row selection with dataview syncGridSelection
- latest slickgrid version also brought the usabilityOverride callback method that was added in RowMoveManager plugin
- possible ref #256
ghiscoding added a commit that referenced this issue Apr 7, 2020
- now create column definition just by the enableRowMoveManager flag
- also keep row selection with dataview syncGridSelection
- latest slickgrid version also brought the usabilityOverride callback method that was added in RowMoveManager plugin
- possible ref #256

Co-authored-by: Ghislain Beaulac <ghislain.beaulac@se.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants