Display column index in table THEAD#240
Conversation
simonwo
left a comment
simonwo
left a comment
There was a problem hiding this comment.
We discussed and:
- I understood that this is the first of 3-4 PRs to add support for source sheet headers, to be used in the validation errors designs. So some of this is temporary and meant to be updated later.
- I clarified that "no header" which is currently used on the sheet selector is probably still a valid design choice, so we may not always want indexes/mapped headers. It's possible that sheet headers are helpful in all cases, and it's also possible that they are only helpful when the user needs to identify cells in the source sheet and are noise otherwise. Designers may want to choose between these options too. So "none", "source" and "mapped" may all be valid options for headers.
| } | ||
|
|
||
| static fromElement(element) { | ||
| if (element.parentElement.tagName == "THEAD"){ |
There was a problem hiding this comment.
Elsewhere uses nodeName… is the difference significant?
There was a problem hiding this comment.
nodeName is typically used for attributes whereas tagName is used for element tag names. nodeName does the right thing for elements, but tagName will break for attributes. I'm not aware of any idioms wrt these two, but if there is one, am happy to change to using it.
| <thead> | ||
| <tr> | ||
| {% for i in range(0, row_length) -%} | ||
| <th>{{ i | encode_header }}</th> | ||
| {% endfor %} | ||
| </tr> | ||
| </thead> |
There was a problem hiding this comment.
The user may make a selection of headers that doesn't include the first N columns:
So we can't assume that the columns start from A:
In the above example the first name column is incorrectly labelled as column A, when in their source spreadsheet it is column C.
In a future PR we'll need to be using the indexes for that column as found in the source spreadsheet if we are going to output them.
There was a problem hiding this comment.
Yeah, I've corrected this in the follow on PR.
| // If we have HTML elements defined on the page (with specific names) then they will be used | ||
| // to store the top left row/column and bottom right row/column. | ||
| if (tlRowTarget) { tlRowTarget.value = selection.startCell.row; } | ||
| // to store the top left row/column and bottom right row/column. | ||
| // We reduce the row index by one to handle the table chrome | ||
| // which should not be included in the selection. | ||
| if (tlRowTarget) { tlRowTarget.value = selection.startCell.row - 1; } | ||
| if (tlColTarget) { tlColTarget.value = selection.startCell.col; } | ||
| if (brRowTarget) { brRowTarget.value = selection.endCell.row; } | ||
| if (brRowTarget) { brRowTarget.value = selection.endCell.row - 1; } | ||
| if (brColTarget) { brColTarget.value = selection.endCell.col; } |
There was a problem hiding this comment.
I guess in a future PR we will need to fix this to not assume that the header is always present.
|
After making progress on #241 I think this PR is only useful for guiding the upcoming UI changes in 241, so I won't merge this, and will just delete once 241 is ready. |
|
Not needed anymore, superceded by #241 |
2 participants
Currently when there is no header name specified, the first row in the table is not shown. To allow users to relate this to the data they can see in their source file, we instead show the column index in base26 (A, B, C ... AA, AB, AC ... etc).
Currently the selectable_table will include this new row in the calculation for the selection, and so we need to make sure we account for it when saving selections.