Summary

Revamp look and feel of the conditions editor.

Review Request #13989 — Created June 19, 2024 and submitted July 1, 2024, 8:18 p.m.

Information

Owner

david

Repository

Djblets

Branch

release-5.x

Bugs

Depends On

Reviewers

Groups

djblets

People

Description

The display of the conditions now uses CSS grid in order to make the
table look more like a table. Widths of those grid columns are
determined by what happens to be visible in each row, instead of having
each row just flow individually with inline-block elements.

Colors use our ink datatable definitions, and icons have been switched
to use the tabler icons from ink instead of ugly colored fontawesome
icons.

As a bonus, I've made it so you can use the keyboard to select and
activate the delete-item entry. This change does not add full
accessibility for these controls, since I'm still trying to wrap my head
around the best practices for some of the things in there.

In order to fully enable a nice look, this also sets some basic styles
for <select> and <option> in configforms. These aren't very
style-able, but this makes it fit in much better.

Testing Done


Edited a bunch of conditions and saw that everything looked correct.
Manually injected the error element into the view and saw that it drew

  much nicer than before (errors are pretty hard to trigger--one needs

  to add an extension that adds condition choices or operators, and then

  disable that extension).
Used the keyboard to edit conditions.
Ran unit tests.
Ran js-tests

Commits

Summary	ID
Revamp look and feel of the conditions editor. The conditions editor widget needed to be updated to support dark mode, because we use it outside of the admin UI for other things. This was all pretty crusty, and while it was tempting to rewrite everything, I've tried to keep my changes somewhat more minimal for now. The display of the conditions now uses CSS grid in order to make the table look more like a table. Widths of those grid columns are determined by what happens to be visible in each row, instead of having each row just flow individually with inline-block elements. Colors use our ink datatable definitions, and icons have been switched to use the tabler icons from ink instead of ugly colored fontawesome icons. As a bonus, I've made it so you can use the keyboard to select and activate the delete-item entry. This change does not add full accessibility for these controls, since I'm still trying to wrap my head around the best practices for some of the things in there. In order to fully enable a nice look, this also sets some basic styles for `<select>` and `<option>` in configforms. These aren't very style-able, but this makes it fit in much better. Testing Done: - Edited a bunch of conditions and saw that everything looked correct. - Manually injected the error element into the view and saw that it drew much nicer than before (errors are pretty hard to trigger--one needs to add an extension that adds condition choices or operators, and then disable that extension). - Used the keyboard to edit conditions. - Ran unit tests. - Ran js-tests Reviewed at https://reviews.reviewboard.org/r/13989/	922b2a2942d69e5705fcdef938e0e9d551dfc480

Summary

Revamp look and feel of the conditions editor.

The conditions editor widget needed to be updated to support dark mode, because we use it outside of the admin UI for other things. This was all pretty crusty, and while it was tempting to rewrite everything, I've tried to keep my changes somewhat more minimal for now. The display of the conditions now uses CSS grid in order to make the table look more like a table. Widths of those grid columns are determined by what happens to be visible in each row, instead of having each row just flow individually with inline-block elements. Colors use our ink datatable definitions, and icons have been switched to use the tabler icons from ink instead of ugly colored fontawesome icons. As a bonus, I've made it so you can use the keyboard to select and activate the delete-item entry. This change does not add full accessibility for these controls, since I'm still trying to wrap my head around the best practices for some of the things in there. In order to fully enable a nice look, this also sets some basic styles for `<select>` and `<option>` in configforms. These aren't very style-able, but this makes it fit in much better. Testing Done: - Edited a bunch of conditions and saw that everything looked correct. - Manually injected the error element into the view and saw that it drew much nicer than before (errors are pretty hard to trigger--one needs to add an extension that adds condition choices or operators, and then disable that extension). - Used the keyboard to edit conditions. - Ran unit tests. - Ran js-tests Reviewed at https://reviews.reviewboard.org/r/13989/

922b2a2942d69e5705fcdef938e0e9d551dfc480

Files

Issues

Description	From	Last Updated
It'd be nice to get the trash can icon aligned if we can. If we align-items or such to baseline, …	chipx86	June 28, 2024, 10:13 p.m.
Looks like alignment regressed here. Also, with align-items: end, what happens when the height on the value widget is tall?	chipx86	July 1, 2024, 9:55 a.m.
This can just be var(--ink-p-red-600) without the --if-dark/light conditionals. That'll still give you the colour for the appropriate palette that's …	maubin	June 20, 2024, 9 a.m.
Let's add role="button" and an aria-label while we're making these changes.	chipx86	June 28, 2024, 9:53 p.m.
We should also add a role="button" here as well.	chipx86	June 28, 2024, 9:54 p.m.
Super-small nit, but maybe move row-gap out from under the grid-* attributes? It really doesn't matter, just makes me go …	chipx86	June 28, 2024, 9:54 p.m.

flake8 passed.

JSHint passed.

djblets/static/djblets/css/forms/conditions.less (Diff revision 1)

The issue has been resolved. Show all issues

This can just be var(--ink-p-red-600) without the --if-dark/light conditionals. That'll still give you the colour for the appropriate palette that's being used.

Commits:

	Summary	ID
	Revamp look and feel of the conditions editor. The conditions editor widget needed to be updated to support dark mode, because we use it outside of the admin UI for other things. This was all pretty crusty, and while it was tempting to rewrite everything, I've tried to keep my changes somewhat more minimal for now. The display of the conditions now uses CSS grid in order to make the table look more like a table. Widths of those grid columns are determined by what happens to be visible in each row, instead of having each row just flow individually with inline-block elements. Colors use our ink datatable definitions, and icons have been switched to use the tabler icons from ink instead of ugly colored fontawesome icons. As a bonus, I've made it so you can use the keyboard to select and activate the delete-item entry. This change does not add full accessibility for these controls, since I'm still trying to wrap my head around the best practices for some of the things in there. Testing Done: - Edited a bunch of conditions and saw that everything looked correct. - Manually injected the error element into the view and saw that it drew much nicer than before (errors are pretty hard to trigger--one needs to add an extension that adds condition choices or operators, and then disable that extension). - Used the keyboard to edit conditions. - Ran unit tests. - Ran js-tests	a62a7abd99153f318dc640f9edc6cbb1a4a23718
	Revamp look and feel of the conditions editor. The conditions editor widget needed to be updated to support dark mode, because we use it outside of the admin UI for other things. This was all pretty crusty, and while it was tempting to rewrite everything, I've tried to keep my changes somewhat more minimal for now. The display of the conditions now uses CSS grid in order to make the table look more like a table. Widths of those grid columns are determined by what happens to be visible in each row, instead of having each row just flow individually with inline-block elements. Colors use our ink datatable definitions, and icons have been switched to use the tabler icons from ink instead of ugly colored fontawesome icons. As a bonus, I've made it so you can use the keyboard to select and activate the delete-item entry. This change does not add full accessibility for these controls, since I'm still trying to wrap my head around the best practices for some of the things in there. Testing Done: - Edited a bunch of conditions and saw that everything looked correct. - Manually injected the error element into the view and saw that it drew much nicer than before (errors are pretty hard to trigger--one needs to add an extension that adds condition choices or operators, and then disable that extension). - Used the keyboard to edit conditions. - Ran unit tests. - Ran js-tests Reviewed at https://reviews.reviewboard.org/r/13989/	7051b66043c620d05214797f7df4d84c2192f32f

Diff:

Revision 2 (+228 -230)

Show changes

	djblets/forms/templates/djblets_forms/conditions_widget.html
	djblets/static/djblets/css/forms/conditions.less
	djblets/static/djblets/js/forms/views/conditionSetView.es6.js
	djblets/static/djblets/js/forms/views/tests/conditionSetViewTests.es6.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Conditions in RB admin UI, light mode (Revision 1)

The issue has been resolved. Show all issues

It'd be nice to get the trash can icon aligned if we can. If we align-items or such to baseline, it might do what we want.

djblets/forms/templates/djblets_forms/conditions_widget.html (Diff revision 2)
The issue has been resolved. Show all issues
```
Let's add role="button" and an aria-label while we're making these changes.
```
djblets/forms/templates/djblets_forms/conditions_widget.html (Diff revision 2)
The issue has been resolved. Show all issues
```
We should also add a role="button" here as well.
```

djblets/static/djblets/css/forms/conditions.less (Diff revision 2)

The issue has been resolved. Show all issues

Super-small nit, but maybe move row-gap out from under the grid-* attributes? It really doesn't matter, just makes me go "gah these should be grouped!".

Commits:

	Summary	ID
	Revamp look and feel of the conditions editor. The conditions editor widget needed to be updated to support dark mode, because we use it outside of the admin UI for other things. This was all pretty crusty, and while it was tempting to rewrite everything, I've tried to keep my changes somewhat more minimal for now. The display of the conditions now uses CSS grid in order to make the table look more like a table. Widths of those grid columns are determined by what happens to be visible in each row, instead of having each row just flow individually with inline-block elements. Colors use our ink datatable definitions, and icons have been switched to use the tabler icons from ink instead of ugly colored fontawesome icons. As a bonus, I've made it so you can use the keyboard to select and activate the delete-item entry. This change does not add full accessibility for these controls, since I'm still trying to wrap my head around the best practices for some of the things in there. Testing Done: - Edited a bunch of conditions and saw that everything looked correct. - Manually injected the error element into the view and saw that it drew much nicer than before (errors are pretty hard to trigger--one needs to add an extension that adds condition choices or operators, and then disable that extension). - Used the keyboard to edit conditions. - Ran unit tests. - Ran js-tests Reviewed at https://reviews.reviewboard.org/r/13989/	7051b66043c620d05214797f7df4d84c2192f32f
	Revamp look and feel of the conditions editor. The conditions editor widget needed to be updated to support dark mode, because we use it outside of the admin UI for other things. This was all pretty crusty, and while it was tempting to rewrite everything, I've tried to keep my changes somewhat more minimal for now. The display of the conditions now uses CSS grid in order to make the table look more like a table. Widths of those grid columns are determined by what happens to be visible in each row, instead of having each row just flow individually with inline-block elements. Colors use our ink datatable definitions, and icons have been switched to use the tabler icons from ink instead of ugly colored fontawesome icons. As a bonus, I've made it so you can use the keyboard to select and activate the delete-item entry. This change does not add full accessibility for these controls, since I'm still trying to wrap my head around the best practices for some of the things in there. Testing Done: - Edited a bunch of conditions and saw that everything looked correct. - Manually injected the error element into the view and saw that it drew much nicer than before (errors are pretty hard to trigger--one needs to add an extension that adds condition choices or operators, and then disable that extension). - Used the keyboard to edit conditions. - Ran unit tests. - Ran js-tests Reviewed at https://reviews.reviewboard.org/r/13989/	5dad4b2f12882d356c712d1741d45b27cad7f691

Diff:

Revision 3 (+232 -230)

Show changes

	djblets/forms/templates/djblets_forms/conditions_widget.html
	djblets/static/djblets/css/forms/conditions.less
	djblets/static/djblets/js/forms/views/conditionSetView.es6.js
	djblets/static/djblets/js/forms/views/tests/conditionSetViewTests.es6.js

Added Files:

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Centered icons (Revision 1)

The issue has been resolved. Show all issues

Looks like alignment regressed here.

Also, with align-items: end, what happens when the height on the value widget is tall?

Description:

		The conditions editor widget needed to be updated to support dark mode,
		because we use it outside of the admin UI for other things. This was all
		pretty crusty, and while it was tempting to rewrite everything, I've
		tried to keep my changes somewhat more minimal for now.

		The display of the conditions now uses CSS grid in order to make the
		table look more like a table. Widths of those grid columns are
		determined by what happens to be visible in each row, instead of having
		each row just flow individually with inline-block elements.

		Colors use our ink datatable definitions, and icons have been switched
		to use the tabler icons from ink instead of ugly colored fontawesome
		icons.

		As a bonus, I've made it so you can use the keyboard to select and
		activate the delete-item entry. This change does not add full
		accessibility for these controls, since I'm still trying to wrap my head
		around the best practices for some of the things in there.
	+
	+	In order to fully enable a nice look, this also sets some basic styles
	+	for `<select>` and `<option>` in configforms. These aren't very
	+	style-able, but this makes it fit in much better.

Commits:

	Summary	ID
	Revamp look and feel of the conditions editor. The conditions editor widget needed to be updated to support dark mode, because we use it outside of the admin UI for other things. This was all pretty crusty, and while it was tempting to rewrite everything, I've tried to keep my changes somewhat more minimal for now. The display of the conditions now uses CSS grid in order to make the table look more like a table. Widths of those grid columns are determined by what happens to be visible in each row, instead of having each row just flow individually with inline-block elements. Colors use our ink datatable definitions, and icons have been switched to use the tabler icons from ink instead of ugly colored fontawesome icons. As a bonus, I've made it so you can use the keyboard to select and activate the delete-item entry. This change does not add full accessibility for these controls, since I'm still trying to wrap my head around the best practices for some of the things in there. Testing Done: - Edited a bunch of conditions and saw that everything looked correct. - Manually injected the error element into the view and saw that it drew much nicer than before (errors are pretty hard to trigger--one needs to add an extension that adds condition choices or operators, and then disable that extension). - Used the keyboard to edit conditions. - Ran unit tests. - Ran js-tests Reviewed at https://reviews.reviewboard.org/r/13989/	5dad4b2f12882d356c712d1741d45b27cad7f691
	Revamp look and feel of the conditions editor. The conditions editor widget needed to be updated to support dark mode, because we use it outside of the admin UI for other things. This was all pretty crusty, and while it was tempting to rewrite everything, I've tried to keep my changes somewhat more minimal for now. The display of the conditions now uses CSS grid in order to make the table look more like a table. Widths of those grid columns are determined by what happens to be visible in each row, instead of having each row just flow individually with inline-block elements. Colors use our ink datatable definitions, and icons have been switched to use the tabler icons from ink instead of ugly colored fontawesome icons. As a bonus, I've made it so you can use the keyboard to select and activate the delete-item entry. This change does not add full accessibility for these controls, since I'm still trying to wrap my head around the best practices for some of the things in there. In order to fully enable a nice look, this also sets some basic styles for `<select>` and `<option>` in configforms. These aren't very style-able, but this makes it fit in much better. Testing Done: - Edited a bunch of conditions and saw that everything looked correct. - Manually injected the error element into the view and saw that it drew much nicer than before (errors are pretty hard to trigger--one needs to add an extension that adds condition choices or operators, and then disable that extension). - Used the keyboard to edit conditions. - Ran unit tests. - Ran js-tests Reviewed at https://reviews.reviewboard.org/r/13989/	922b2a2942d69e5705fcdef938e0e9d551dfc480

Diff:

Revision 4 (+298 -232)

Show changes

	djblets/forms/templates/djblets_forms/conditions_widget.html
	djblets/static/djblets/css/config-forms.less
	djblets/static/djblets/css/forms/conditions.less
	djblets/static/djblets/js/forms/views/conditionSetView.es6.js
	djblets/static/djblets/js/forms/views/tests/conditionSetViewTests.es6.js

Added Files:

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-5.x (1f7ea2b)