Opened 10 years ago

Closed 9 years ago

#9287 closed enhancement (fixed)

Dojo UE Features - Phase 1

Reported by: Evan Owned by: Nathan Toone
Priority: high Milestone: 1.4
Component: DojoX Grid Version: 1.3.0
Keywords: ibm oci grid ue Cc: Nathan Toone, drs
Blocked By: Blocking:

Description (last modified by Adam Peller)

The following features are added for base Grid:

  1. Nested sorting
  2. Indirect selection
  3. Context menu
  4. Swipe selection and DnD moving columns & rows

please refer to the design spec in http://docs.google.com/Doc?id=ddt3wpr4_298c9mtpqfr, and implementation spec in http://docs.google.com/Doc?id=dhht2z92_1c4jpgkcd

Attachments (8)

dojo-enhanced-grid-090630.patch (168.6 KB) - added by Adam Peller 10 years ago.
latest dojox.grid patch from Evan
enhanced-grid-support.zip (5.9 KB) - added by Adam Peller 10 years ago.
supporting image/test files from Evan. See README.txt
dojo-enhanced-grid-090730.patch (186.4 KB) - added by Adam Peller 10 years ago.
latest patch from Evan
dojo-enhanced-grid-081709.patch (196.7 KB) - added by Adam Peller 10 years ago.
update from Evan
nestedSortArrows.png (955 bytes) - added by Adam Peller 10 years ago.
goes in dojox/grid/enhanced/resources/images
EDG-memory-testing.jpg (43.0 KB) - added by bill 9 years ago.
memory leak test results
dojo-enhanced-grid-#9287-08182010-2.patch (114.1 KB) - added by bill 9 years ago.
updated patch from evan
EDG-memory-testing-results-openclose.jpg (102.0 KB) - added by evan 9 years ago.

Download all attachments as: .zip

Change History (38)

comment:1 Changed 10 years ago by Adam Peller

Description: modified (diff)

To apply Evan's patch:

  1. Apply "dojo-grid-ue-090512.patch" to dojox directory
  2. Put *.png under {DOJO}/dojox/grid/resources/images/
  3. Put "music-for-demo.part.csv" under {DOJO}/dojox/grid/tests/support/

Changed 10 years ago by Adam Peller

latest dojox.grid patch from Evan

Changed 10 years ago by Adam Peller

Attachment: enhanced-grid-support.zip added

supporting image/test files from Evan. See README.txt

comment:2 Changed 10 years ago by Adam Peller

Cc: drs added

latest update from Evan (2009-07-26) includes:

  1. Removed all changes to Base Grid (by extending Builder - thanks to Nathan's new changes)
  2. Refined nls loading for Nested Sorting
  3. Fixed many a11y issues(keyboard focus) & other misc defects.
  4. Changed the location of testing cases
  5. Added more comments

The test case for EnhancedGrid? is at dojox/grid/tests/enhanced/test_enhanced_grid.html

Changed 10 years ago by Adam Peller

latest patch from Evan

comment:3 Changed 10 years ago by Adam Peller

Cc: Nathan Toone added

090730 patch:

  1. Removed unused css classes in tundraEnhancedGrid.css
  2. Used _setHeaderMenuAttr() like methods to avoid deprecation warning
  3. Cleaned the code a bit
  4. Added the "music-for-demo.part.csv" in the patch

comment:4 Changed 10 years ago by Nathan Toone

Milestone: tbd1.4
Owner: changed from Bryan Forbes to Nathan Toone

Reassigning to me for tracking, since I've been working more with Evan....

comment:5 Changed 10 years ago by Nathan Toone

I am not able to apply the latest patch. Would you mind creating a more recent one. I think that we are about to the point where we will be wanting to check some stuff into svn...

Changed 10 years ago by Adam Peller

update from Evan

Changed 10 years ago by Adam Peller

Attachment: nestedSortArrows.png added

goes in dojox/grid/enhanced/resources/images

comment:6 Changed 10 years ago by Adam Peller

latest patch from Evan at dojo-enhanced-grid-081709.patch (see also nestedSortArrows.png)

  • Fixed some issues of programmatic row selection
  • Added keyboard support for moving rows/columns (via Ctrl + Arrow keys)
  • Synchronized with latest Dojo trunk
  • Fixed several defects (DnD rendering, visual design related ...)

comment:7 Changed 10 years ago by Adam Peller

(In [19901]) Initial patch from Evan Huang (IBM) for enhanced grid. Refs #9287 !strict

comment:8 Changed 10 years ago by Adam Peller

(In [19911]) from Evan: added feature description to test case. Refs #9287

comment:9 Changed 10 years ago by Adam Peller

(In [19912]) from Evan: added feature description to test case. Refs #9287

comment:10 Changed 10 years ago by Adam Peller

(In [19934]) refactor enhanced grid css, from Evan. Refs #9287

comment:11 Changed 10 years ago by Adam Peller

(In [19935]) style fixes from Evan. Refs #9287 !strict

comment:12 Changed 10 years ago by Adam Peller

Drag/drop comments: I was surprised that dnd worked only on the selected portion and not on the column header/row header. Also, I expected the drop to occur where the mouse pointer was. It seems to use the top/left of the dragged box instead.

comment:13 Changed 10 years ago by David Schwartz

There is a visual indication of the drop location during a drag. Also, as column selection is persistent, there needs to be a way of disambiguating column selection and drag. Hence, drags are initiated from the content area, not the header.

comment:14 Changed 10 years ago by Adam Peller

I'm suggesting that the visual indication is not correct. And for column selection, is there a drag operation on the column header? If not, I don't see why disambiguation is necessary.

comment:15 Changed 10 years ago by Adam Peller

the drop should be effective based on where the mouse is, not where the top-left corner is. I'm not sure how else to say it.

comment:16 Changed 10 years ago by David Schwartz

I understand your point re the mouse position; seems like a reasonable argument. I'm investigating....

RE the drag issue, column header drag events are consumed by the swipe select feature.

comment:17 Changed 10 years ago by Adam Peller

I suggest we open up separate tickets for features like swipe select (most features, actually) Swipe select currently works from headers only and not from cells.

comment:18 Changed 10 years ago by Adam Peller

Resolution: fixed
Status: newclosed

See #9919 for swipe select #9920 for drag/drop behavior

Closing ticket, since this patch was applied. Open up new issues in separate tickets.

comment:19 Changed 10 years ago by Adam Peller

(In [20512]) Patch from Evan- fixes incorrect column mover width and style/spelling corrections. Refs #9287 !strict

comment:20 Changed 10 years ago by Adam Peller

(In [20530]) From Evan. Refs #9287 !strict

  1. Some formatting & spelling
  2. Make the mover height more accurate for multiple views
  3. Removed redundant 'tundraEnhancedGrid_rtl.css'(all the rtl fixes are in 'EnhancedGrid_rtl.css').

comment:21 Changed 10 years ago by Adam Peller

(In [21664]) Various enhanced grid fixes from Evan, including:

  • IE6 paging performance issue by fixing measurePage()
  • No onSelected/onDeselected events will be fired after sorting
  • Added keepSortSelection (false by default) which is used to persist selection across sortings (only applicable for client-side data stores)

Refs #9287 !strict

comment:22 Changed 9 years ago by bill

Adding patch from Evan that fixes the following issues in EDG:

1.Memory leak

  • Empty all connect/topics during destroy() phase
  • Memory issues are fixed with this patch (pls see the attached "EDG-memory-testing.jpg") -

2.DnD API improvement

  • Mapping info is add in published topic when col/row moved in form of {fromIdx: toIdx}

3.Requirement - Make the dummy plugin mechanism more pluggable in EDG

Changed 9 years ago by bill

Attachment: EDG-memory-testing.jpg added

memory leak test results

Changed 9 years ago by bill

updated patch from evan

comment:23 Changed 9 years ago by Bryan Forbes

Could you post memory results after closing the browser window/tab the grid is running in? That will give us a real measure of leaks. Right now, it's just showing us how much resident memory is taken up by the page. Make sure you disable Firebug when testing Firefox because there is a slight leak when using FB.

comment:24 Changed 9 years ago by evan

Resolution: fixed
Status: closedreopened

Sure, Bryan,

I tried that and is attaching the result "EDG-memory-testing-results-openclose.jpg", also changed this to "reopen" as we still have the following TODO items:

1.Update campus doc

2.Clean up the test cases

3.Investigate any ways to leverage both the indirect selection and Grid's _Selector

4.Keep improving performance & internal structure as possible.

Changed 9 years ago by evan

comment:25 Changed 9 years ago by Bryan Forbes

After an very cursory look over this patch, I see the following problems:

  1. This code:
    dojo.hitch(this.grid.selection, dojox.grid.Selection.prototype[value ? 'addToSelection' : 'deselect'])(idx);
    
    Can be simplified to:
    dojox.grid.Selection.prototype[value ? 'addToSelection' : 'deselect'].call(this.grid.selection, idx);
    
    Please change any code that uses the first pattern use the second pattern.
  1. The indirect selection code is still using a widget per row. Imagine 1,000 rows of data and you scroll through all 1,000 rows. This means that you will now have 1,000 instances of whatever widget you are using for the indirect selection.

I'll keep looking at the codebase and keep providing feedback.

comment:26 Changed 9 years ago by evan

Thanks Bryan,

Actually #2 won't happen with this patch except user explicitly set "keepRows=1000" - widgets will be destroyed whenever their page gets destroyed. So the number of widgets exists at the same time == rowsPerPage * existed page number.

This patch is targeting 1.5.1 by addressing most urgent issues. I'm starting working on a separate patch to leverage indirect selection and _Selector way to fundamentally improve the performance, will post that once ready.

comment:27 Changed 9 years ago by Adam Peller

In general, it's good to reuse tickets when we can, but at some point, we really need new tickets to track these defects, especially as they're going into patch releases.

comment:28 Changed 9 years ago by evan

Got it, so we will track this with a new one #11633

comment:29 Changed 9 years ago by Adam Peller

sounds like three independent things. normally I'd suggest three tickets, but let's see how this goes.

comment:30 Changed 9 years ago by Adam Peller

Resolution: fixed
Status: reopenedclosed

new issues will be tracked in separate tickets

Note: See TracTickets for help on using tickets.