rollover style for editable cells in ListGrid

[senordhuff]

Hi there, we are hoping to implement a grid where the user sees a visual cue that a field is editable when mousing over a cell. If you look at this example:

https://www.smartclient.com/smartcli...?id=editByCell

The editable cells show a blue border when you click on the editable cell. We want to display a border like that when the user mouses over the cell to give a visual cue that the cell is editable just by mousing over the cell. What is the best way to achieve that?

[senordhuff]

Hi, I've gotten part of my task complete like this:

        getBaseStyle:function (record, rowNum, colNum){ 

            var baseStyle = this.Super("getBaseStyle", arguments);

            if(this.canEditCell(rowNum,colNum)  ) {
                baseStyle = 'editableATCell';                        
            }

            return baseStyle;            

        },

However, that logic applies the style of the editableATCell css class to all editable cells in a row when you mouse over any cell in a row. I can't yet figure out how to make the style only apply to the cell where the mouse currently has rolled over. I feel like I'm missing something basic. Any suggestions?

[Isomorphic]

You can determine what cell the mouse is over via getEventRow() / getEventColumn(). Either getBaseStyle() or getCellCSSText() could be override points to add code like the above, plus the addition of checking the mouseover cell.

[senordhuff]

Thank you for the response. That is helpful. So, now that I've included getEventRow and getEventColumn checks into my implementation, it is much closer to complete. Here is the latest code

        getBaseStyle:function (record, rowNum, colNum){ 

            var baseStyle = this.Super("getBaseStyle", arguments);


            if(this.canEditCell(rowNum,colNum) && this.getEventRow()==rowNum && this.getEventColumn()==colNum  ) {
                baseStyle = 'editableATCell';                        
            }

            return baseStyle;            

        },

The only remaining issue I see is that getBaseStyle only seems to fire when the mouse first enters any cell in the row. As you move the mouse cursor from cell to cell in the same row, getBaseStyle() is not firing. So, the code above only executes when you switch from row to row but not from cell to cell on the same row. Any ideas on how to fix that issue?

[Isomorphic]

You'll need to add a mouseMove handler that calls refreshCell() when moving over editable cells where you want styling to change on mouseOver.

[senordhuff]

Thanks again. I have this working now. It actually requires a call to refreshRow() in order to ensure that only the current cell gets the editable style. Here is the implementation

        mouseMove : function () {

            this.refreshRow(this.getEventRow());

        },

        getBaseStyle:function (record, rowNum, colNum){ 

            var baseStyle = this.Super("getBaseStyle", arguments);

            if(this.canEditCell(rowNum,colNum) && this.getEventRow()==rowNum && this.getEventColumn()==colNum  ) {
                baseStyle = 'editableATCell';                        
            }

            return baseStyle;            

        },

I noticed there is a refreshCellStyle method which I assume is faster than refreshCell. But, since we are calling refrreshRow do you think it would be faster to iterate over all of the cells in the row and call refreshCellStyle on each them individually? Or, is there no performance penalty to just call refreshRow() like you see above?

[Isomorphic]

refreshRow() refreshes the entire row, so it's equivalent to calling refreshCell() on every cell in the row. You're also just calling it on every mouseMove, even if the cursor is within the same cell or row, so that's extremely inefficient.

You should be refreshing cells only, and refreshing only when the mouse moves from one cell to another, and only for cells where you are actually changing the styling, not other cells.

Having gotten that massive efficiency implemented, yes, refreshCellStyle() is a little faster than refreshCell() for your use case.

[senordhuff]

Thanks for saving me from massively inefficient code! Does this look better below?

        mouseMove : function () {

            if(this.currentEventRow!=null && this.currentEventColumn!=null){

                //only call refreshCellStyle() on new and old cells if mouseMove caused a change in the eventRow or eventColumn
                if(this.getEventRow()!=this.currentEventRow || this.getEventColumn()!=this.currentEventColumn){

                    //new cell entered, refresh new cell
                    this.refreshCellStyle(this.getEventRow(),this.getEventColumn());

                    //old cell exited, refresh old cell
                    this.refreshCellStyle(this.currentEventRow ,this.currentEventColumn);
                }

            }else{

                //current state variables not set yet, refresh cell
                this.refreshCellStyle(this.getEventRow(),this.getEventColumn());

            }

            //set state variables on each mouse move
            this.currentEventRow = this.getEventRow();
            this.currentEventColumn = this.getEventColumn();


        },

        getBaseStyle:function (record, rowNum, colNum){ 

            var baseStyle = this.Super("getBaseStyle", arguments);
            if(this.canEditCell(rowNum,colNum) && this.getEventRow()==rowNum && this.getEventColumn()==colNum  ) {
                baseStyle = 'editableATCell';                        
            }
            return baseStyle;            

        },

[Isomorphic]

You're missing a check that the cell is an editable cell - if it's not, it doesn't need to be refreshed. Same with the cell just exited.

But this is a minor thing as it's not very important that cell transitions are faster in non-editable areas.

[senordhuff]

Ok, good point. here is the final code I believe

        mouseMove : function () {

            if(this.currentEventRow!=null && this.currentEventColumn!=null){

                //only call refreshCellStyle() on new and old cells if respective cells are editable and mouseMove caused a change in the eventRow or eventColumn
                if(this.getEventRow()!=this.currentEventRow || this.getEventColumn()!=this.currentEventColumn){

                    if(this.canEditCell(this.getEventRow(),this.getEventColumn())){
                        //new cell entered, refresh new cell
                        this.refreshCellStyle(this.getEventRow(),this.getEventColumn());                        
                    }

                    if(this.canEditCell(this.currentEventRow,this.currentEventColumn)){
                        //old cell exited, refresh old cell
                        this.refreshCellStyle(this.currentEventRow ,this.currentEventColumn);                        
                    }
                }

            }else{

                if(this.canEditCell(this.getEventRow(),this.getEventColumn())){
                    //current state variables not set yet, refresh cell
                    this.refreshCellStyle(this.getEventRow(),this.getEventColumn());                    
                }

            }

            //set state variables on each mouse move
            this.currentEventRow = this.getEventRow();
            this.currentEventColumn = this.getEventColumn();


        },

        getBaseStyle:function (record, rowNum, colNum){ 

            var baseStyle = this.Super("getBaseStyle", arguments);
            if(this.canEditCell(rowNum,colNum) && this.getEventRow()==rowNum && this.getEventColumn()==colNum  ) {
                baseStyle = 'editableATCell';                        
            }
            return baseStyle;            

        },

[senordhuff]

Just posting a final update in case this is ever useful to someone else. The canEditCell() calls seem to erroneously return true for rows and columns less than 0 which is not theoretically possible. This happens when moving the mouse below the grid data for instance. Here is a final version that protects against that by checking that eventrow and eventcolumn are greater than 0 before calling canEditCell

        mouseMove : function () {

            if(this.lastEventRow!=null && this.lastEventColumn!=null){

                //only call refreshCellStyle() on new and last cells if respective cells are editable and mouseMove caused a change in the eventRow or eventColumn
                if(this.getEventRow()!=this.lastEventRow || this.getEventColumn()!=this.lastEventColumn){

                    if(this.getEventRow()>=0 && this.getEventColumn()>=0 && this.canEditCell(this.getEventRow(),this.getEventColumn())){

                        //new cell entered, refresh new cell
                        this.refreshCellStyle(this.getEventRow(),this.getEventColumn());                        
                    }

                    if(this.lastEventRow>=0 && this.lastEventColumn>=0 && this.canEditCell(this.lastEventRow,this.lastEventColumn)){

                        //old cell exited, refresh last cell
                        this.refreshCellStyle(this.lastEventRow ,this.lastEventColumn);                        
                    }
                }

            }else{

                if(this.getEventRow()>=0 && this.getEventColumn()>=0 && this.canEditCell(this.getEventRow(),this.getEventColumn())){

                    //current state variables not set yet, refresh cell
                    this.refreshCellStyle(this.getEventRow(),this.getEventColumn());                    
                }

            }

            //set lastEvent state variables on each mouse move
            this.lastEventRow = this.getEventRow();
            this.lastEventColumn = this.getEventColumn();


        },

[senordhuff]

Hi Isomoprhic, now that this code in production, we are getting reports of users seeing errors because this.frozenFields it not defined in getEventColumn(). We have created the following patch you should consider adding to the framework

             getEventColumn : function (x) {
                 var tempX = (x != null ? x : this.getOffsetX()),
                     colNum = 0,
                     frozenColumns = 0
                 ;

                 //patch to check for frozenFields too
                 if (this.frozenBody && this.frozenFields) frozenColumns = this.frozenFields.length;

                 if (frozenColumns > 0 && tempX < this.frozenBody.getVisibleWidth()) {
                     colNum = this.frozenBody.getEventColumn(x);
                 } else {
                     colNum = this.body.getEventColumn(x) + frozenColumns;
                 }

                 return colNum;
             }

[Isomorphic]

Hi
This is interesting - in normal usage the framework sets up the frozenBody when fields are frozen and clears it when fields are unfrozen, so we wouldn't expect to encounter a situation where this null check is required.

As a sanity check it's perfectly safe, and we would consider adding it to the framework as requested, but we would be interested in understanding how users are getting their grids into the state where this is required in the first place.

If there's any way you could show us steps to reproduce state on either one of the standard sample grids or your own sample code, we'd be very interested to see it in action.

Regards
Isomorphic Software

[senordhuff]

We get the error reporting from our customer usage showing this error but have unfortunately not been able to recreate it yet. It has only happened a few times suggesting it is a corner case.