flake8
passed.
JSHint
passed.
Create a field state label view.
Review Request #13242 — Created Aug. 31, 2023 and submitted
This change adds a view for a field state label, a pill shaped label that
displays the state of a field, like our Draft/Submitted/Discarded state labels
for review requests in the dashboard. This can be used on review request
fields to show which fields have been modified in a review request draft.
We'll make use of the label in an upcoming change for file attachments, to
help display which file attachments are new, new revisions, drafts, or
pending deletion.Right now the label is pretty simple, but it could be expanded upon in the
future to allow a button for reverting a field, diffing the change of a
field, etc. We currently have draft and deleted color themes for the label.
- Used in the upcoming file attachments change.
- Attached the label to the summary field.
| Summary |
|---|
| Description | From | Last Updated |
|---|---|---|
|
I know this is technically in another change, but to avoid shifting the layout, could we overlay the top-left of … |
 chipx86
|
|
|
Please add a doc comment at the top describing the file. |
 david
|
|
|
This needs a semicolon. |
david
|
|
|
Is this something that we expect to inherit from and override? If not, I don't think we need a template … |
david
|
|
|
Type is wrong here. |
david
|
|
|
Because this view is templatized, we should probably type this as TExtraViewOptions |
david
|
|
|
Can you move this to rb/ui/field-state-label.less? Aiming to give every component its own file. |
chipx86
|
|
|
Let's use ems here. |
chipx86
|
|
|
Each color (and other units) we reference in the component should be defined in a #rb-ns-ui.field-state-label() namespace. Those can then … |
chipx86
|
|
|
Naming of constants should be all UPPER_CASE. |
chipx86
|
-
-
reviewboard/static/rb/js/ui/views/fieldStateLabelView.ts (Diff revision 1) Please add a doc comment at the top describing the file.
-
-
reviewboard/static/rb/js/ui/views/fieldStateLabelView.ts (Diff revision 1) Is this something that we expect to inherit from and override? If not, I don't think we need a template here, and can just do
.text(this.#state)in the render function. -
-
reviewboard/static/rb/js/ui/views/fieldStateLabelView.ts (Diff revision 1) Because this view is templatized, we should probably type this as
TExtraViewOptions
Change Summary:
- Made the view not templatized since just setting the text of the label will suffice for now.
Commits: |
|
||||||
|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+312) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
-
-
I know this is technically in another change, but to avoid shifting the layout, could we overlay the top-left of the file attachment thumbnail, offsetting into it maybe 0.5em or something?
-
reviewboard/static/rb/css/pages/review-request.less (Diff revision 2) Can you move this to
rb/ui/field-state-label.less? Aiming to give every component its own file. -
-
reviewboard/static/rb/css/pages/review-request.less (Diff revision 2) Each color (and other units) we reference in the component should be defined in a
#rb-ns-ui.field-state-label()namespace. Those can then map to the color codes or other constants. That way, there's one place to update to start changing these values (which will be important later as we move to dark mode support). -
reviewboard/static/rb/js/ui/views/fieldStateLabelView.ts (Diff revision 2) Naming of constants should be all UPPER_CASE.
Change Summary:
- Makes use of a
#rb-ns-ui.field-state-label()namespace. - Moved to the CSS for the field state label to
rb/ui/review-request/field-state-label.less.
Commits: |
|
||||||
|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+378) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.