updated styling for security container

Review Request #11175 — Created Sept. 18, 2020 and updated

ruonanjia
Review Board
release-4.0.x
4881
reviewboard, students

Updated styling for security center

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

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

Summary Author
updated security center styling to match other pages
ruonan
removed extra padding and indentation
ruonan
remove legacy class for form
ruonan
Loading file attachments...

Description From Last Updated

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

daviddavid

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.

daviddavid

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

hailanhailan

For your summary, please capitalize the first letter.

daviddavid

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

daviddavid

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

daviddavid

We didn't used to have a lot of standards around the way things looked in the admin UI (historical technical ...

chipx86chipx86

"form-row" is considered a legacy class, so we should probably stop using it. There are newer classes for lists of ...

chipx86chipx86

Please switch these so they're in alphabetical order.

daviddavid

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

daviddavid
ruonanjia
ruonanjia
  1. Ship It!

  2. 
      
ruonanjia
ruonanjia
ruonanjia
ruonanjia
david
  1. 
      
  2. Can you take a screenshot of the updated page and attach it using "Update > Add File"?

  3. 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.

  4. 
      
ruonanjia
ruonanjia
chipx86
  1. 
      
  2. 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:

    1. 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.
    2. There's a max width enforced, but there's not much reason for that. Let's get rid of that.
    3. 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.
  3. "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.

    1. 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 :)

  4. 
      
ruonanjia
ruonanjia
hailan
  1. 
      
  2. The lines in Description and Testing Done are more than 70 characters ;)

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

    2. 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.

    3. nvm got it!

  3. 
      
ruonanjia
ruonanjia
Review request changed

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

ruonanjia
  1. 
      
  2. 
      
david
  1. 
      
  2. For your summary, please capitalize the first letter.

  3. 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.

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

  5. Please switch these so they're in alphabetical order.

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

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

  7. 
      
Loading...