Fix active cell update on tabbing #18614
Conversation
Pull Request Test Coverage Report for Build 2116775063
💛 - Coveralls |
src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.html
Outdated
Show resolved
Hide resolved
src/sql/workbench/contrib/notebook/browser/cellViews/code.component.ts
Outdated
Show resolved
Hide resolved
It is still wonky when tabbing backwards. It doesn't toggle backward across all the elements like it does forward, it just loops between first two elements backward. Still looking into that. |
src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.html
Outdated
Show resolved
Hide resolved
src/sql/workbench/contrib/notebook/browser/notebook.component.ts
Outdated
Show resolved
Hide resolved
src/sql/workbench/contrib/notebook/browser/notebook.component.ts
Outdated
Show resolved
Hide resolved
| @@ -116,6 +117,11 @@ export class CellModel extends Disposable implements ICellModel { | |||
| } | |||
| // if the fromJson() method was already called and _cellGuid was previously set, don't generate another UUID unnecessarily | |||
| this._cellGuid = this._cellGuid || generateUuid(); | |||
| if (this._cellType === 'code') { | |||
| this.cellLabel = localize('codeCellLabel', "Code Cell {0}", this.id); | |||
There was a problem hiding this comment.
I don't think ID is appropriate here since that's just a random number. Generally for lists of objects like this you would announce the index and then the label (which would just be "Code Cell" or "Markdown Cell"). But getting this working correctly is likely out of scope for this PR so I guess this is fine - just putting this here for future reference.
Although for code cells we should probably be saying the language as well.
|
Tested and looks good! I found an issue when entering a code cell editor, the cell's edit mode is not changed unless I tab again and the contents are modified. Also, the changes in this pr fixes this issue as well #18504 |
|
@barbaravaldez Could you log an issue for the code cell issue you mentioned, looks like it's a different issue than what am addressing here. Also can you check if it's regression ? Thanks!! |
|
If it's not a regression (it's an issue with this PR) then I don't think a separate issue is necessary. That should be fixed before this goes in. |
@MaddyDev this is not a regression because we didn't change the cell edit mode when tabbing before |
But am not doing anything with the edit mode here, am just setting the cell as active on focus. Can you explain me more whats the expected experience and what's the regression you're seeing? |
This PR addresses part of the issue #18274, it only fixes the tabbing forward scenario.