Port PageView to spina.
Review Request #12937 — Created April 5, 2023 and submitted
Information | |
---|---|
david | |
Review Board | |
release-6.x | |
Reviewers | |
reviewboard | |
This change ports the base PageView class to spina and TypeScript. As
part of this, the code has been moved into thecommon
subdirectory to
match the bundle that it's a part of.The one change that was required outside of this class was to update the
DiffViewerPageModel
to usepreinitialize
instead ofconstructor
.
The use of the reserved nameconstructor
has bitten us in the past,
and wasn't working anymore now that the base class was an ES6 class.
- Ran js-tests.
- Verified that page views still rendered correctly.
Summary | |
---|---|
Description | From | Last Updated |
---|---|---|
Since this was accessible on the prototype, but isn't really an instance variable, I wonder if we should make this … |
|
|
Can we work to document all instance variables going forward? |
|
|
We can return super.remove() instead. |
|
|
Since this has multiple types involved, let's move actionId to its own line, so the :'s don't all run together. |
|
|
Are you able to just import 'jasmine-core' and get the same results? I've resolved linting locally with just an import … |
|
-
-
reviewboard/static/rb/js/common/views/pageView.ts (Diff revision 1) Since this was accessible on the prototype, but isn't really an instance variable, I wonder if we should make this
static
and include it in@spina({prototypeAttrs: [...]})
? -
reviewboard/static/rb/js/common/views/pageView.ts (Diff revision 1) Can we work to document all instance variables going forward?
-
reviewboard/static/rb/js/common/views/pageView.ts (Diff revision 1) We can
return super.remove()
instead. -
reviewboard/static/rb/js/common/views/pageView.ts (Diff revision 1) Since this has multiple types involved, let's move
actionId
to its own line, so the:
's don't all run together. -
reviewboard/static/rb/js/common/views/tests/pageViewTests.ts (Diff revision 1) Are you able to just
import 'jasmine-core'
and get the same results? I've resolved linting locally with just animport 'jasmine'
, and wasn't sure if it'd work the same way.