Mobile Diff Viewer (Cosmetic Changes)

Review Request #9396 - Created Nov. 25, 2017 and updated

Theodore Brockman
Review Board
master
3532717...
reviewboard, students
  • Removed review request information from the diff section (leaving it in the reviews section)
  • Made diff index anchor list collapsable
  • Fixed the display of the revision table file list on mobile
  • Changed the styling of the view control buttons on mobile
  • Changed the styling of the pending review banner on mobile

Added Javascript tests to ensure functionality of collapsing anchor list,
visual inspection and manual-testing of other changes.

Loading file attachments...

  • 1
  • 0
  • 8
  • 5
  • 14
Description From Last Updated
This banner still needs to be fixed, but is not included in this review request. Theodore Brockman Theodore Brockman
Checks run (2 failed)
flake8 failed.
JSHint failed.

flake8

JSHint

Theodore Brockman
Review request changed

Change Summary:

Removing unnecessarily tracked files.

Summary:

-Fix #4492. Deprecated "review_request.get_close_description" and extended with "review_request.close_info", which returns a dictionary containing the values previously returned by "get_close_description", in addition to a datetime object representing the time the review request was closed.
+Mobile Diff Viewer Cosmetic Changes

Description:

~  

Fixing indent level to please ReviewBot, also fixing previously added undefined reference.

  ~

Just some pretty things.

-  
-  

-  
-  

Rename closed date view variables, changed additional variable creation into direct dictionary accesses.

-  
-  

-  
-  

before plane

-  
-  

-  
-  

Merge branch 'master' of https://github.com/reviewboard/reviewboard

-  
-  

-  
-  

Moving cosmetics to a separate feature branch.

-  
-  

-  
-  

Moved less variable definitions to where they belong.

-  
-  

-  
-  

working on cosmetic changes, commit before merge

-  
-  

-  
-  

Merge branch 'master' of https://github.com/reviewboard/reviewboard into diffViewerCosmetics

-  
-  

-  
-  

test commit

Commit:

-b1555f037a246eca38f3b4fb75d0b53e935fd34b
+acdae096df838bf7e2438d348d21e92aaf4f6f02

Diff:

Revision 2 (+332 -38)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 passed.
JSHint failed.

JSHint

Theodore Brockman
Review request changed
Theodore Brockman
Theodore Brockman
Theodore Brockman
Mukhtar Alhejji
  1. 
      
  2. This line should be less than 78 characters

  3. This line should be less than 78 characters

  4. This line should be less than 78 characters

  5. This line should be less than 78 characters

  6. 
      
Theodore Brockman
Graham Sea
  1. Cosmetic curiosity

  2. For change 5 why did you go with blue instead of the standard green?

    1. You could make arguments for different colors based on the context of the banner. Am I being warned that I have a pending review that requires immediate action (yellow/red)? Is it a successful operation that requires no input from me (green)? Is it non-urgent information (blue)?

      But to be honest, it was a pretty arbitrary decision. I tried it with the original green but I felt it clashed with the line addition color scheme of the diffs (because they're slightly different greens). Changing the banner color to the line addition green didn't look that great either.

      I went with blue, because I liked the blue. If others feel differently I am more than okay to change it to whatever.

  3. 
      
Theodore Brockman
Theodore Brockman
Theodore Brockman
Theodore Brockman
Review request changed

Description:

   
  • Removed review request information from the diff section (leaving it in the reviews section)
   
  • Made diff index anchor list collapsable
   
  • Fixed the display of the revision table file list on mobile
   
  • Changed the styling of the view control buttons on mobile
   
  • Changed the styling of the pending review banner on mobile
  +
  • Fixed interactions with the revision selector on touchscreens

Commit:

-f13b51b613d8077b855d2222f1991e3899f39919
+5133c2f8c9ab867527dacea22840633cde87930f

Diff:

Revision 7 (+367 -41)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 passed.
JSHint failed.

JSHint

Theodore Brockman
Theodore Brockman
  1. 
      
  2. These changes should probably just be in a separate review request with a test making sure the touch screen events are handled correctly

  3. 
      
Theodore Brockman
Theodore Brockman
Theodore Brockman
Theodore Brockman
Review request changed
Theodore Brockman
  1. 
      
  2. Publish banner

    This banner still needs to be fixed, but is not included in this review request.

  3. 
      
Loading...