- Download Diff

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	Author
Updated security center styling to match other pages	ruonan

Screenshots

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	Author
-	updated styling for security container at admin/security	ruonan
+	updated security center styling to match other pages	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	Author
-	updated security center styling to match other pages	ruonan
+	updated security center styling to match other pages	ruonan
+	removed extra padding and indentation	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	Author
-	updated security center styling to match other pages	ruonan
-	removed extra padding and indentation	ruonan
+	updated security center styling to match other pages	ruonan
+	removed extra padding and indentation	ruonan
+	remove legacy class for form	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	Author
-	updated security center styling to match other pages	ruonan
-	removed extra padding and indentation	ruonan
-	remove legacy class for form	ruonan
+	updated security center styling to match other pages	ruonan
+	Updated security center styling to match other pages	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	Author
-	updated security center styling to match other pages	ruonan
-	Updated security center styling to match other pages	ruonan
+	Updated security center styling to match other pages	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

You have a pending review.

Review Board 5.0.2 alpha 0 (dev)

updated styling for security container

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