Skip to content

Commit fe5843e

Browse files
PaulL1PaulL1
committed
Enh(doco): create design folder, put some thoughts in it
1 parent 9fb9605 commit fe5843e

File tree

3 files changed

+293
-4
lines changed

3 files changed

+293
-4
lines changed

‎design/EDIT_AND_CELLNAV.md

+159
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
# Edit and CellNav, Focus in the Grid
2+
3+
Both edit and cellNav provide ways to focus particular cells in the grid. This, and the associated
4+
`editOnFocus` option have been a source of pain, but they are also (unfortunately) a very desirable
5+
feature that we'd like to have working smoothly.
6+
7+
The current implementation is inspired by the google sheets implementation. A key feature is that
8+
the focus is held by the viewport, not by the individual cells, and focus is "faked" through use of
9+
css classes and directing keypresses to particular elements.
10+
11+
<< @swalters: is this only true for cellNav, or is it also true for edit? It seems that edit must
12+
be receiving focus or else some of the custom editors just wouldn't work >>
13+
14+
We continue to have difficulties with the interaction of these two modules, and with the behaviour
15+
upon events such as scrolling, scrolling that is triggered by typing (for a part-visible cell),
16+
`editOnFocus` and whether it works reliably.
17+
18+
This overview aims to document what we think our business features are, and then describe a framework
19+
that we could move to that would make the logic cleaner and therefore easier to fix defects in.
20+
21+
## CellNav
22+
23+
The intention of cellNav is that a user can select a cell, and then use the keyboard to move around
24+
the grid. When the focus is given to a particular cell the grid should attempt to scroll to make that
25+
cell fully visible (if it's not already fully visible). If the cell is too tall or too wide to be
26+
fully visible, then the grid should show as much of the cell as possible whilst keeping the top left
27+
corner fully visible.
28+
29+
Keyboard actions that should move the focused cell are:
30+
- 4 arrow keys
31+
- pg up, pg down (move a page at a time)
32+
- enter (moves down)
33+
- shift+enter (moves up)
34+
- tab (moves right)
35+
- shift+tab (moves left)
36+
37+
Where a navigation hits the edge of the grid, the navigation should "wrap around". If we move off
38+
a row to the right, we should come back on 1 row down, in the leftmost focusable cell. If we move
39+
off a row to the left, we should come back 1 row up, in the rightmost focusable cell. Similarly for
40+
moving up and down - if we hit enter till we get to the bottom of the grid, we should re-enter at the
41+
top.
42+
43+
There are two exceptions to this behaviour. When we exit the top left focusable cell, we should shift focus
44+
off the grid entirely, to the previous widget on the page. When we exit the bottom right focusable cell, we should
45+
shift focus to the next widget on the page.
46+
47+
When we tab into the grid from another widget on the page, we should select the top left or bottom right
48+
focusable cell.
49+
50+
When we scroll we should remember which cell had focus. If that cell remains visible then we should highlight
51+
it as having the focus. If that cell scrolls off the page then we should remember it, and highlight it again
52+
when it comes visible.
53+
54+
A source of contention is that when we give a cell focus, we attempt to make it visible. When a user manually
55+
scrolls we reset the focus to the correct cell, but we don't want to automatically scroll to make it visible - this
56+
will give jankiness to our scrolling as the focused cell moves off the page.
57+
58+
Another contention is that as we tab across a row we sometimes get scroll up and down for some reason to do with
59+
native browser behaviour.
60+
61+
62+
## Edit
63+
64+
Edit works by noticing that a cell has entered edit mode, and replacing the cell contents with the specified editor.
65+
The editor will raise events for start_edit, end_edit and cancel_edit. Upon end or cancel the edit feature will
66+
remove the editor and replace it with the normal cell template.
67+
68+
`editOnFocus` is a feature that works like a spreadsheet - when you enter a cell the editor is displayed and the
69+
cell is immediately editable.
70+
71+
Upon a cell becoming editable we should attempt to scroll it so that it is fully visible, and as with cellNav if we
72+
cannot make it fully visible we should do as good a job as we can whilst keeping the top left corner visible.
73+
74+
Editors often have default navigation actions. For example, in an input box if you use the arrow keys to go to the
75+
edge of the field and then off, it will shift focus to the next field. This is effectively a cellNav.
76+
77+
Our different editors have slightly different desired default behaviours.
78+
79+
### Input
80+
81+
Upon an input field receiving focus all the text in that field should be automatically selected, the cell should
82+
be scrolled so far as possible to make the field fully visible. The arrow keys when pressed should remove the highlight
83+
of all text, and move around within the field. They should not exit the field when they reach the end.
84+
85+
Enter, shift+enter, tab, shift+tab should end editing, and should trigger a cellNav. Perhaps these should still be
86+
trapped by cellNav itself, and not by edit? Is this possible with standard html5 widgets?
87+
88+
## Date
89+
90+
Ideally this would one day be a proper date picker. Anyway, up and down arrow keys should adjust the current
91+
date element we're on (the year, month or day), left and right arrow keys should select the next date part in that
92+
direction. They should not take us off the field to the left or right.
93+
94+
Enter, shift+enter, tab, shift+tab should end editing, and should trigger a cellNav. Perhaps these should still be
95+
trapped by cellNav itself, and not by edit? Is this possible with standard html5 widgets?
96+
97+
## Dropdown
98+
99+
Starting edit should drop down the dropdown. Up and down arrows should move the selection. Left and right
100+
arrows should move the selection the same as up and down.
101+
102+
Enter, shift+enter, tab, shift+tab should end editing, and should trigger a cellNav. Perhaps these should still be
103+
trapped by cellNav itself, and not by edit? Is this possible with standard html5 widgets?
104+
105+
## Custom editors
106+
107+
In general custom editors should follow the pattern of only triggering a cellNav on Enter, shift+enter, tab, shift+tab,
108+
this also means that custom editors should not use these keys for anything else.
109+
110+
## Deep edit
111+
112+
I can't remember why this is a good idea. It seems that the semantics of the input field work fine without a deep edit
113+
concept. I think it's more that when an editable field gets focus but hasn't yet entered edit mode, we still let the
114+
left/right work with cellNav. So maybe deep edit is sort of a halfway house of `editOnFocus` that we don't really need?
115+
116+
## Edit On Focus
117+
118+
Edit on focus should work that whenever a cell gets focus we should immediately enter edit mode. Otherwise the
119+
behaviour should remain the same.
120+
121+
## Problems
122+
123+
Edit has a number of points of contention with cellNav, and the interoperation of the two is problematic (but also
124+
essential). Areas of conflict are:
125+
126+
- when you're typing in a cell you can trigger a scroll if the cell isn't fully visible. The scroll may change
127+
which DOM elements are what, and therefore which DOM element it is that we're editing in. The upshot is that
128+
we tend to lose focus on the editor, and then gain focus again. When we gain focus again we're sometimes not
129+
where we used to be - for example on the input field we may select all the text rather than returning our cursor
130+
to where it was. These are usability issues for a user who is typing in the field
131+
132+
- some editors propagate events to the parent object, for example the standard input seems to propagate a right
133+
arrow key at the end of the field to the parent, which in turn causes a cellNav. This isn't our desired
134+
behaviour
135+
136+
- if the user scrolls whilst a field is in edit mode we sometimes get unusual behaviour. Really we want it to work
137+
similarly to if a scroll happened whilst editing a field - we want our position in the editor at the end of the scroll
138+
to be the same as at the start, unless our field has gone off page, in which case we want to end edit mode
139+
140+
- edit on focus is hard because when a cell gets focus we start edit. Starting edit then attempts to select all the
141+
text, and scroll so we're visible, which makes a mess
142+
143+
## Design thoughts
144+
145+
Can we agree that tab, shift+tab, enter, shift+enter are always trapped by cellNav, and that all editors must stay
146+
away from them? Can we agree that other than those 4 keys, no editor will ever propagate navigation (and work out
147+
how to make that true, as well as document how to do it for custom editors as well)?
148+
149+
Can we define some sort of state callback for each editor (that each editor must create and respond to)? So
150+
when a scroll happens we'd call getState on the editor, then when the scroll completes we'd find the new instance
151+
of that editor and call setState? Then each editor is responsible for maintaining it's own state, or choosing not
152+
to maintain it. For some editors there may be no state at all (for a dropdown, the selected item is the state), for
153+
others it's pretty basic (for input it's the cursor location and the selected text start and end).
154+
155+
Can we draw a distinction between getting focus through a user action (keyboard action or mouse click or touch), and
156+
re-getting focus after a scroll or other grid rendering change? Then we could choose not to select all the text
157+
and scroll to make a cell visible if we're re-getting focus as opposed to initially getting focus?
158+
159+

‎misc/api/design-rendering-cycle.ngdoc ‎design/RENDERING_CYCLE.md

+1-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
@ngdoc overview
2-
@name Rendering Cycle
3-
@module
4-
@description
1+
# Grid Rendering Cycle
52

63
The core grid rendering cycle is executed whenever the grid needs re-rendering. There are a number of core methods,
74
with some of those methods able to be called individually.

‎design/SCROLLING_AND_VIRTUALISATION.md

+133
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
# Scrolling and Virtualisation Design
2+
3+
This document is a first cut at documenting how scrolling and virtualisation works, and
4+
providing a path to refactoring or otherwise tidying up the scrolling logic.
5+
6+
## Overview
7+
8+
We have various scrollable renderContainers: left, body and right. The body
9+
render container is always present, the left and right render containers depend
10+
on RTL settings and pinning settings. In a left to right (default) setup with
11+
grouping or selection enabled you'll have a left render container that holds the
12+
row header (the select buttons or the grouping expand buttons).
13+
14+
Within each render container we have a header, a viewport and a footer.
15+
16+
The viewport within the render containers scrolls vertically, with the scroll bar displayed
17+
in the rightmost render container. Irrespective of which render container receives the
18+
scroll event we scroll all containers that are present vertically (scroll synchronisation).
19+
20+
The body render container scrolls horizontally, with the header, viewport and footer all scrolling.
21+
At the current time the scrollbar is shown at the bottom of the body viewport, consideration should
22+
be given as to whether it should instead be shown at the bottom of the footer render container. Irrespective
23+
of whether the header, viewport or footer receive the scroll event, all three are synchronised (again,
24+
scroll synchronisation).
25+
26+
Virtualisation is implemented to reduce the number of DOM elements rendered. If a grid has 10,000 visible
27+
rows we provide the illusion that we have rendered all of them, but we actually only create DOM elements
28+
for those rows that are currently visible, and a reasonable buffer either side. Similarly for horizontal
29+
scrolling we only render the columns that are actually visible plus a buffer either side. We then apply
30+
padding to the viewports to provide the illusion that there are more DOM elements rendered than there are.
31+
32+
Virtualisation applies to the body renderContainer (header, viewport, footer) for horizontal scrolling, and
33+
to the left, right and body viewports for vertical scrolling.
34+
35+
36+
## Design Concept
37+
38+
### Separation
39+
40+
We separate scrolling from virtualisation.
41+
- Scrolling is the process of moving the visible area of a canvas
42+
- Virtualisation is the process of deciding whether to change the DOM elements that are rendered, and
43+
associated padding
44+
45+
46+
### Scrolling
47+
48+
Scrolling should try to behave like native browser scrolling as far as possible - when a user triggers a
49+
scroll using a scrollbar, trackpad or touch event the only additional processing we should add is to
50+
synchronise the relevant containers, based on the scroll directions.
51+
52+
Scrolling should provide events to virtualisation so that we know that scrolling occurred, but the act of
53+
scrolling should not be blocked by virtualisation - this should reduce any janking or speed issues. Ideally
54+
scrolling on it's own does not trigger a digest cycle - digests should only be triggered by virtualisation, if
55+
at all.
56+
57+
### Virtualisation
58+
59+
Virtualisation is the act of deciding which DOM elements should be currently rendered, and applying padding
60+
to make the scrollbars look as if there's the right amount of scroll distance. We render the visible rows
61+
plus `excessRows` additional rows to the top and bottom. We render the visible columns plus `excessColumns`
62+
additional columns to the left and right. When virtualisation is notified of a scroll event we take a look
63+
to see whether we have at least `minExcessRows` of buffer in the scroll direction, and at least `minExcessCols`
64+
of buffer in the scroll direction.
65+
66+
If either minExcess is breached, then we adjust the DOM such that we have `excessRows` and `excessColumns` again,
67+
and adjust the scroll padding appropriately.
68+
69+
In order to avoid browser blocking and general thrashing, we implement a throttle. Once one scroll event finishes
70+
we always wait at least `minScrollWait` ms before we respond to another scroll event. At the point we do respond,
71+
we respond based on the current scroll position, not based on any intermediate positions.
72+
73+
74+
### Variable Row and Column Size
75+
76+
In implementing this design concept we'll also explicitly allow for variable height rows and columns. We'll build
77+
logic that iterates the visible column array and the visible row array, and adds to each row and column a `scrollToHere`
78+
value that tells us the pixel count to the leftmost edge of the column, or the topmost edge of the row.
79+
80+
The height/width attributed to a row/column will be based on a height/width that is stored directly on the row/column,
81+
if that is not present then it will base off the rowHeight set in the gridOptions.
82+
83+
The `scrollToHere` value will need to be recalculated based on triggers. The notifyDataChange architecture
84+
will be used to manage these triggers. Triggers will be:
85+
- column resize
86+
- row height change
87+
- rerun of rowsProcessors or columnsProcessors (i.e. potentially change in ordering, visible rows/cols etc)
88+
- change of data (perhaps picked up in the above)
89+
- change of columnDefs (perhaps picked up in the above)
90+
91+
92+
### Watchers
93+
94+
We have watchers on the row and column / on the cell that look for a change in the row/col that this particular
95+
DOM element is attached to.
96+
97+
Column changes are automatically handled through use of `track by uid`, which means that any time the uid of a
98+
column changes new DOM elements will be rendered. Having said this, it isn't currently clear that this means
99+
we can avoid the watchers, but essentially this should mean that we never reuse DOM elements for columns - any
100+
new uid will result in new DOM elements. For rows, the trigger for reuse is virtualisation. This means that
101+
we can replace the current watchers with a notification architecture instead, and we can specifically notify
102+
when we change the row/DOM alignment. (If it turns out we also want watchers for columns, we could similarly
103+
notify of changes to the col).
104+
105+
We would do this using the notifyDataChange architecture. This would remove a number of watchers.
106+
107+
108+
### Events
109+
110+
What events do we have, and what are they used for? Need to ask @swalters or read the code.
111+
112+
113+
### Benefits
114+
115+
The result should be that slow scrolling is very smooth - most scroll ticks will result in the canvas
116+
moving but no DOM re-rendering and no digest cycle. When a user scrolls faster we may result in every scroll tick
117+
meeting the criteria for re-rendering the DOM, but we'll only do the actual re-render at a maximum rate per second,
118+
so we're not driving into digest hell and thrashing our browser (particularly on lower powered devices such as
119+
mobile devices).
120+
121+
122+
### Things to Watch Out For
123+
124+
The virtualisation / scrolling cycle currently calls one of the variants of grid refresh (perhaps refreshCanvas,
125+
can't recall off the top of my head). It's also likely that one of the grid refresh variants will need to recalculate
126+
virtualisation, as this would be the trigger for some things like data having changed. We need to take care that we
127+
don't create an endless loop.
128+
129+
Scrolling is a bit untidy in the movement between grid triggered and native browser scrolls. I seem to recall that
130+
we get native scroll events triggering when we internally scroll, we need to be careful to not end up in a loop,
131+
and we can perhaps manage CPU use by not processing both. Perhaps we can have a way to detect that they originate
132+
with the same user event?
133+

0 commit comments

Comments
 (0)