Summary

updated styling for security container

Review Request #11175 — Created Sept. 18, 2020 and discarded Dec. 6, 2020, 7:23 p.m.

Information

Owner

ruonanjia

Repository

Review Board

Branch

master

Bugs

4881

Depends On

Reviewers

Groups

reviewboard, students

People

Description

This change makes the styling for the security page more consistent with
the other pages.

Uses rounded borders with white background instead of black solid border.

Testing Done

Manually tested that styles are updated and looking correct when running
reviewboard locally.

Commits

Summary	ID	Author
Updated security center styling to match other pages	c7c000aed3e5d4a8f0a2a3110d0cd97b0512bcd0	ruonan

Files

Issues

Description	From	Last Updated
Can you take a screenshot of the updated page and attach it using "Update > Add File"?	david	Sept. 18, 2020, 1:47 p.m.
Please see https://www.notion.so/reviewboard/Writing-Good-Change-Descriptions-10529e7c207743fa8ca90153d4b21fea for info on how we'd like to see the summary/description written.	david	Sept. 18, 2020, 1:54 p.m.
The lines in Description and Testing Done are more than 70 characters ;)	hailan	Oct. 14, 2020, 12:55 p.m.
For your summary, please capitalize the first letter.	david	Dec. 1, 2020, 10 p.m.
The first line of your description can be removed, since it's the same as your summary. Please also add punctuation …	david	Dec. 1, 2020, 10:01 p.m.
Mind uploading a new version of the screenshot so we can see what it looks like now?	david	Dec. 1, 2020, 10:19 p.m.
The posted diff has a bunch of unrelated stuff. Please pull the latest master and rebase your change.	david	Dec. 2, 2020, 1:24 p.m.
We didn't used to have a lot of standards around the way things looked in the admin UI (historical technical …	chipx86	Sept. 29, 2020, 4:21 p.m.
"form-row" is considered a legacy class, so we should probably stop using it. There are newer classes for lists of …	chipx86	Sept. 29, 2020, 4:49 p.m.
Please switch these so they're in alphabetical order.	david	Dec. 1, 2020, 10:04 p.m.
Because you removed one level of the tags here, you'll need to dedent the <ul> and everything inside it by …	david	Dec. 1, 2020, 10:19 p.m.

flake8 passed.

JSHint passed.

Status: Closed (submitted)

```
Ship It!
```

Status: Re-opened

Groups:

+	students

Description:

~		updated styling for security container to match new styles at path admin/security
	~	updated styling for security center to match other pages
	+
	+	switched fieldset to a div with classNames using new styles

Testing Done:

Manually tested that styles are updated and looking right when running reviewboard locally

Description:

		updated styling for security center to match other pages
-
-		switched fieldset to a div with classNames using new styles

Show all issues

Can you take a screenshot of the updated page and attach it using "Update > Add File"?

Show all issues

Please see https://www.notion.so/reviewboard/Writing-Good-Change-Descriptions-10529e7c207743fa8ca90153d4b21fea for info on how we'd like to see the summary/description written.

Description:

~		updated styling for security center to match other pages
	~	Updated styling for security center
	+
	+	This change makes the styling for the security page more consistent with the other pages
	+
	+	It uses rounded borders with white background instead of black solid border

Commits:

	Summary	ID	Author
	updated styling for security container at admin/security	0993395494fc1bf8e3b18a37cad3037f271fd4f0	ruonan
	updated security center styling to match other pages	f9bbb614c77d039619b6b2775ad93adf302edcc9	ruonan

Diff:

Revision 2 (+4 -4)

Show changes

reviewboard/templates/admin/security.html

Added Files:

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Description:

		Updated styling for security center

		This change makes the styling for the security page more consistent with the other pages

~		It uses rounded borders with white background instead of black solid border
	~	Uses rounded borders with white background instead of black solid border

Screen Shot 2020-09-18 at 1.41.11 PM.png (Revision 1)

Show all issues

We didn't used to have a lot of standards around the way things looked in the admin UI (historical technical reasons for that). We can do better now, and really clean this up. A few things immediately stand out here:

Too much padding around this, compared to other pages. Likely coming from the existing security center classes, or default element styles. We can probably fix this up by adding some better rules for the classes being used.
There's a max width enforced, but there's not much reason for that. Let's get rid of that.
It'd be nice to have that indented part have the same level of "indentation" as the outer container, once the above is fixed up. We actually have LessCSS constants for all this, so we can keep things nice and consistent.

reviewboard/templates/admin/security.html (Diff revision 2)

Show all issues

"form-row" is considered a legacy class, so we should probably stop using it. There are newer classes for lists of items, like what's used in the Extensions or Integrations pages, and it might be interesting to see how it looks using those.

ruonanjia Sept. 29, 2020, 4:50 p.m.

tried adding djblets-c-config-forms-list__item classes but it did not make a noticeable difference to how it looked. Let me know if it's still preferable to add them in :)

Commits:

Summary	ID	Author
updated security center styling to match other pages	f9bbb614c77d039619b6b2775ad93adf302edcc9	ruonan
updated security center styling to match other pages	f9bbb614c77d039619b6b2775ad93adf302edcc9	ruonan
removed extra padding and indentation	2abb29027e726c88614dfbb842ac3e4c7b5aab36	ruonan

Diff:

Revision 3 (+13 -9)

Show changes

	reviewboard/static/rb/css/pages/admin/security-center.less
	reviewboard/templates/admin/security.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commits:

Summary	ID	Author
updated security center styling to match other pages	f9bbb614c77d039619b6b2775ad93adf302edcc9	ruonan
removed extra padding and indentation	2abb29027e726c88614dfbb842ac3e4c7b5aab36	ruonan
updated security center styling to match other pages	f9bbb614c77d039619b6b2775ad93adf302edcc9	ruonan
removed extra padding and indentation	2abb29027e726c88614dfbb842ac3e4c7b5aab36	ruonan
remove legacy class for form	2658ed6681c29c8c1c7cc2eb01e8bcb10f595bbf	ruonan

Diff:

Revision 4 (+12 -14)

Show changes

	reviewboard/static/rb/css/pages/admin/security-center.less
	reviewboard/templates/admin/security.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Show all issues

The lines in Description and Testing Done are more than 70 characters ;)

MarcusBoay Oct. 1, 2020, 10:17 p.m.

You're right but just to clarify, according to this document, the lines should be less than 80 characters, not 70.

ruonanjia Oct. 14, 2020, 12:07 p.m.

Do you guys mean summary? I see that summary needs to be under 80 characters but I don't see a character restriction on description and testing done.

ruonanjia Oct. 14, 2020, 12:55 p.m.
```
nvm got it!
```

Summary:

-	updated styling for security container at admin/security
+	updated styling for security container

Description:

		Updated styling for security center

~		This change makes the styling for the security page more consistent with the other pages
	~
	+
	+	This change makes the styling for the security page more consistent with
	+	the other pages
	+
	+

		Uses rounded borders with white background instead of black solid border

Testing Done:

~		Manually tested that styles are updated and looking right when running reviewboard locally
	~	Manually tested that styles are updated and looking right when running
	+	reviewboard locally

Show all issues

For your summary, please capitalize the first letter.

Show all issues

The first line of your description can be removed, since it's the same as your summary. Please also add punctuation to the description and testing done.

Show all issues

Mind uploading a new version of the screenshot so we can see what it looks like now?

ruonanjia Dec. 1, 2020, 10:26 p.m.
```
added
```

reviewboard/static/rb/css/pages/admin/security-center.less (Diff revision 4)
Show all issues
```
Please switch these so they're in alphabetical order.
```

reviewboard/templates/admin/security.html (Diff revision 4)

Show all issues

Because you removed one level of the tags here, you'll need to dedent the <ul> and everything inside it by one space.

Description:

-		Updated styling for security center
-
-
-
		This change makes the styling for the security page more consistent with
~		the other pages
	~	the other pages.
-
-

~		Uses rounded borders with white background instead of black solid border
	~	Uses rounded borders with white background instead of black solid border.

Testing Done:

~		Manually tested that styles are updated and looking right when running
~		reviewboard locally
	~	Manually tested that styles are updated and looking correct when running
	~	reviewboard locally.

Commits:

Summary	ID	Author
updated security center styling to match other pages	f9bbb614c77d039619b6b2775ad93adf302edcc9	ruonan
removed extra padding and indentation	2abb29027e726c88614dfbb842ac3e4c7b5aab36	ruonan
remove legacy class for form	2658ed6681c29c8c1c7cc2eb01e8bcb10f595bbf	ruonan
updated security center styling to match other pages	583be5c72ec924f674d860bcb905626242386aba	ruonan
Updated security center styling to match other pages	714c659cd0d98eb3ce0f6cc2979bb098017dc7c7	ruonan

Branch:

-	release-4.0.x
+	master

Diff:

Revision 5 (+186 -463)

Show changes

	AUTHORS
	contrib/internal/prepare-dev.py
	reviewboard/settings.py
	reviewboard/admin/server.py
	reviewboard/attachments/mimetypes.py
	reviewboard/cmdline/rbsite.py
	reviewboard/hostingsvcs/github.py
	reviewboard/static/rb/css/pages/admin/security-center.less
	5 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Removed Files:
Added Files:

Commits:

Summary	ID	Author
updated security center styling to match other pages	583be5c72ec924f674d860bcb905626242386aba	ruonan
Updated security center styling to match other pages	714c659cd0d98eb3ce0f6cc2979bb098017dc7c7	ruonan
Updated security center styling to match other pages	c7c000aed3e5d4a8f0a2a3110d0cd97b0512bcd0	ruonan

Diff:

Revision 6 (+210 -485)

Show changes

	AUTHORS
	contrib/internal/prepare-dev.py
	reviewboard/settings.py
	reviewboard/admin/server.py
	reviewboard/attachments/mimetypes.py
	reviewboard/cmdline/rbsite.py
	reviewboard/hostingsvcs/github.py
	reviewboard/static/rb/css/pages/admin/security-center.less
	5 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Fix it!

Show all issues

The posted diff has a bunch of unrelated stuff. Please pull the latest master and rebase your change.

ruonanjia Dec. 6, 2020, 7:24 p.m.

moved to https://reviews.reviewboard.org/r/11316/
Branch was messed up

Review Board 6.0.3 alpha 0 (dev)

updated styling for security container

Information

Reviewers

Status: Closed (submitted)

Status: Re-opened

Groups:

Description:

Testing Done:

Description:

Description:

Commits:

Diff:

Added Files:

Checks run (2 succeeded)

Description:

Commits:

Diff:

Checks run (2 succeeded)

Commits:

Diff:

Checks run (2 succeeded)

Summary:

Description:

Testing Done:

Description:

Testing Done:

Commits:

Branch:

Diff:

Checks run (2 succeeded)

Removed Files:

Added Files:

Commits:

Diff:

Checks run (2 succeeded)

Status: Discarded