Summary

Create a field state label view.

Review Request #13242 — Created Aug. 31, 2023 and submitted Sept. 28, 2023, 6:41 p.m.

Information

Owner

maubin

Repository

Review Board

Branch

release-6.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

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.

Testing Done


Used in the upcoming file attachments change.
Attached the label to the summary field.

Commits

Summary	ID
Create a field state label view.	88d8712051ca8026d4bd43ffb79e8603eeea9e38

Files

Issues

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	Sept. 12, 2023, 8:21 p.m.
Please add a doc comment at the top describing the file.	david	Sept. 11, 2023, 4:15 p.m.
This needs a semicolon.	david	Sept. 11, 2023, 4:19 p.m.
Is this something that we expect to inherit from and override? If not, I don't think we need a template …	david	Sept. 11, 2023, 4:22 p.m.
Type is wrong here.	david	Sept. 11, 2023, 4:22 p.m.
Because this view is templatized, we should probably type this as TExtraViewOptions	david	Sept. 11, 2023, 4:27 p.m.
Can you move this to rb/ui/field-state-label.less? Aiming to give every component its own file.	chipx86	Sept. 20, 2023, 5:36 a.m.
Let's use ems here.	chipx86	Sept. 12, 2023, 8:02 p.m.
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	Sept. 12, 2023, 8:02 p.m.
Naming of constants should be all UPPER_CASE.	chipx86	Sept. 12, 2023, 6:47 p.m.

flake8 passed.

JSHint passed.

reviewboard/static/rb/js/ui/views/fieldStateLabelView.ts (Diff revision 1)
The issue has been resolved. Show all issues
```
Please add a doc comment at the top describing the file.
```
reviewboard/static/rb/js/ui/views/fieldStateLabelView.ts (Diff revision 1)
The issue has been resolved. Show all issues
```
This needs a semicolon.
```

reviewboard/static/rb/js/ui/views/fieldStateLabelView.ts (Diff revision 1)

The issue has been resolved. Show all issues

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)
The issue has been resolved. Show all issues
```
Type is wrong here.
```
reviewboard/static/rb/js/ui/views/fieldStateLabelView.ts (Diff revision 1)
The issue has been dropped. Show all issues
```
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:

	Summary	ID
	Create a field state label view.	671636ac9f2eb6a36e202c1ce21a32223e2b4219
	Create a field state label view.	6ace585d9cf6fe95ba92542155d3d47fbee9fd0d

Diff:

Revision 2 (+312)

Show changes

	reviewboard/static/rb/css/pages/review-request.less
	reviewboard/static/rb/js/ui/index.ts
	reviewboard/static/rb/js/ui/views/fieldStateLabelView.ts

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Deleted Theme (Revision 1)

The issue has been resolved. Show all issues

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)

The issue has been resolved. Show all issues

Can you move this to rb/ui/field-state-label.less? Aiming to give every component its own file.

maubin Sept. 12, 2023, 8:02 p.m.

Would rb/ui/review-request/field-state-label.less be more appropriate since the field state label should only apply to review request pages?

chipx86 Sept. 18, 2023, 11:37 p.m.
```
Sure, that works.
```

reviewboard/static/rb/css/pages/review-request.less (Diff revision 2)
The issue has been resolved. Show all issues
```
Let's use ems here.
```

reviewboard/static/rb/css/pages/review-request.less (Diff revision 2)

The issue has been resolved. Show all issues

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)
The issue has been resolved. Show all issues
```
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:

	Summary	ID
	Create a field state label view.	6ace585d9cf6fe95ba92542155d3d47fbee9fd0d
	Create a field state label view.	88d8712051ca8026d4bd43ffb79e8603eeea9e38

Diff:

Revision 3 (+378)

Show changes

	reviewboard/static/rb/css/bundles/reviews.less
	reviewboard/static/rb/css/pages/review-request.less
	reviewboard/static/rb/css/ui/review-request/field-state-label.less
	reviewboard/static/rb/js/ui/index.ts
	reviewboard/static/rb/js/ui/views/fieldStateLabelView.ts

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-6.x (c487dea)