- Download Diff

Summary:

[WIP] Add Show/Hide .min button for minified source files

Review Request #7700 — Created Oct. 15, 2015 and discarded Nov. 23, 2015, 3:24 p.m.

Information
Owner:	eklau
Repository:	Review Board
Branch:	master
Bugs:
Depends On:
Reviewers
Groups:	reviewboard
People:

Description

In order to determine if the file is a minified file, I have to change the file in the database so that the will end with .min.css, .min.js, or .min.less.
First, I went to localhost:8080. Then I went to the top right bar, under my username I clicked into admin. Then, I went to database near the top left navigation bar, under DIFFVIEWER, I clicked into file diff and changed various file names to file.min.js and file.min.css and file.min.less on both source and destination file to test.
Now there are some files with keywords ending with ".min.", I can test to modify files with those substring.

-10/19
I am trying to figure out how the diffReviewableView.js is connected with diff_file_fragment.html to link the onclick events with the button. 
I also need to know what class the contents are being wrapped around so that I can show/hide those contents when I click the button.

-10/24
The new Show/Hide .min button can now show and hide the diff-box. The location of the button is now updated. I added the button to where the other expand/collapse buttons were for managing data within the diff-box. I still need to only show the new button for .min.css, .min.js, and .min.less files.

-10/31
I have to add a function in the python code so that it will recognize that the file name is a minified file. I'm still working on understanding the Python source code for whitespace so that I can have a similar message for the minified file. I need to hide the contents of the minified files by default, and have a link that when pressed, will show those contents.

-11/6
The diff review page now shows the message to detect that the file is a minified file. I still have to change the css for the message (currently uses whitespace class). I also need to make hide the contents of the minified file by default and create a link that will show the contents of the file when pressed.

-11/14

Testing Done

Test string inputs to see where they are being displayed

-10/19
Test button to see where they are displayed. Test button functionality with alert()

Test button functionality with diffReviewable.js

-10/24
Add button location with view_diff.html

Test button functionality with diffViewerPageView.js

Remove diff-highlight when .min file is hidden

Change font-weight of Show/Hide .min button to normal weight

-10/31
Working on understanding python source code for whitespace, no code updated yet

-11/6
Add new member of the dictionary in diff_utils.py

Add .min. file message in diff_file_fragment.html

Test min-link class in diffReviewable.js

Remove old button and it's functions

-11/14

Files

Issues

Description	From	Last Updated
This is the header I mean. When "hidden", this should still appear and the table body should say something like: …	brennie	Nov. 14, 2015, 11:05 a.m.
HTML comments end with -->, not --!>. In this case, the browser is guessing what you meant to do, but …	david	Oct. 30, 2015, 7:07 p.m.
It looks like there is an extre newline here.	SH sherman	Nov. 14, 2015, 11:05 a.m.
I think these change comments are not very helpful. All changes are already highlighted in the diff viewer and git …	SH sherman	Nov. 14, 2015, 11:04 a.m.
We do not want to set the block to be invisible. We still want to see the header describing the …	brennie	Nov. 14, 2015, 11:05 a.m.
mf is not descriptive enough a selector.	brennie	Nov. 14, 2015, 11:04 a.m.

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js

The summary here "WIP" could do with the project name as well even if it's a WIP. At the moment, it forces someone to do some digging to find out what the project is.
This might be helpful: https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/

chipx86 Oct. 17, 2015, 3:09 p.m.

Indeed, we'll want a detailed description even for WIP review requests.

Summary:

-	WIP
+	Finding the correct place to insert view minified code button.

Testing Done:

~		WIP
	~	There is no code written yet, I have only tested string inputs to see where they are being displayed.

Edward, can you add "WIP" to the summary of this, so we know it's work in progress? Thanks!

Summary:

-	Finding the correct place to insert view minified code button.
+	Finding the correct place to insert view minified code button. WIP.

Testing Done:

~		There is no code written yet, I have only tested string inputs to see where they are being displayed.
	~	WIP. There is no code written yet, I have only tested string inputs to see where they are being displayed.

Description:

		In order to determine if the file is a minified file, I have to change the file in the database so that the will end with .min.css, .min.js, or .min.less.

		First, I went to localhost:8080. Then I went to the top right bar, under my username I clicked into admin. Then, I went to database near the top left navigation bar, under DIFFVIEWER, I clicked into file diff and changed various file names to file.min.js and file.min.css and file.min.less on both source and destination file to test.

~		Now there are some files with keywords ending with ".min.", I can test to modify files with those substring.
	~	Now there are some files with keywords ending with ".min.", I can test to modify files with those substring.
	+	-10/19
	+
	+	I am trying to figure out how the diffReviewableView.js is connected with diff_file_fragment.html so that I can link the onclick events with the button.
	+
	+	I also need to know what class the contents are being wrapped around so that I can show/hide those contents when I click the button.
	+	-10/24

Testing Done:

~		WIP. There is no code written yet, I have only tested string inputs to see where they are being displayed.
	~	WIP.
	+
	+	There is no code written yet, I have only tested string inputs to see where they are being displayed.
	+	-10/19
	+
	+	A button is added on the inside the diffviewer table in the file diff_file_fragment.html. It currently has an alert upon clicking, which is supposed to hide/show the contents in the diffviewer instead.
	+
	+	I have tried to implement some code to test the function in the diffReviewableView.js, but currently it doesn't seem to do anything.
	+	-10/24

Added Files:

Diff:

Revision 2 (+13 -2)

Show changes

	reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
	reviewboard/templates/diffviewer/diff_file_fragment.html

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/templates/diffviewer/diff_file_fragment.html



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/templates/diffviewer/diff_file_fragment.html

Why did you attach screenshots of source code?

reviewboard/templates/diffviewer/diff_file_fragment.html (Diff revision 2)

Show all issues

HTML comments end with -->, not --!>. In this case, the browser is guessing what you meant to do, but the way you have it is messing up syntax highlighting.

Diff:

Revision 3 (+13 -2)

Show changes

	reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
	reviewboard/templates/diffviewer/diff_file_fragment.html

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/templates/diffviewer/diff_file_fragment.html



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/templates/diffviewer/diff_file_fragment.html

Diff:

Revision 4 (+13 -2)

Show changes

	reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
	reviewboard/templates/diffviewer/diff_file_fragment.html

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/templates/diffviewer/diff_file_fragment.html



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/templates/diffviewer/diff_file_fragment.html

Summary:

-	Finding the correct place to insert view minified code button. WIP.
+	[WIP] Add Show/Hide .min button for minified source files

Description:

		In order to determine if the file is a minified file, I have to change the file in the database so that the will end with .min.css, .min.js, or .min.less.

		First, I went to localhost:8080. Then I went to the top right bar, under my username I clicked into admin. Then, I went to database near the top left navigation bar, under DIFFVIEWER, I clicked into file diff and changed various file names to file.min.js and file.min.css and file.min.less on both source and destination file to test.

		Now there are some files with keywords ending with ".min.", I can test to modify files with those substring.
		-10/19

~		I am trying to figure out how the diffReviewableView.js is connected with diff_file_fragment.html so that I can link the onclick events with the button.
	~	I am trying to figure out how the diffReviewableView.js is connected with diff_file_fragment.html to link the onclick events with the button.

		I also need to know what class the contents are being wrapped around so that I can show/hide those contents when I click the button.
		-10/24
	+
	+	The new Show/Hide .min button can now show and hide the diff-box. The location of the button is now updated. I added the button to where the other expand/collapse buttons were for managing data within the diff-box. I still need to only show the new button for .min.css, .min.js, and .min.less files.
	+	-10/31

Testing Done:

~		WIP.
	~	Test string inputs to see where they are being displayed
-
-		There is no code written yet, I have only tested string inputs to see where they are being displayed.
		-10/19

~		A button is added on the inside the diffviewer table in the file diff_file_fragment.html. It currently has an alert upon clicking, which is supposed to hide/show the contents in the diffviewer instead.
~
	~	Test button to see where they are displayed. Test button functionality with alert()
	~	Test button functionality with diffReviewable.js
-		I have tried to implement some code to test the function in the diffReviewableView.js, but currently it doesn't seem to do anything.
		-10/24
	+
	+	Add button location with view_diff.html
	+	Test button functionality with diffViewerPageView.js
	+	Remove diff-highlight when .min file is hidden
	+	Change font-weight of Show/Hide .min button to normal weight
	+	-10/31

Diff:

Revision 5 (+50 -3)

Show changes

	reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
	reviewboard/static/rb/js/pages/views/diffViewerPageView.js
	reviewboard/templates/diffviewer/diff_file_fragment.html
	reviewboard/templates/diffviewer/view_diff.html

Removed Files:

Added Files:

Tool: Pyflakes
Ignored Files:
    reviewboard/templates/diffviewer/view_diff.html
    reviewboard/templates/diffviewer/diff_file_fragment.html
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/static/rb/js/pages/views/diffViewerPageView.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/templates/diffviewer/view_diff.html
    reviewboard/templates/diffviewer/diff_file_fragment.html
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/static/rb/js/pages/views/diffViewerPageView.js

afterMinClick.png (Revision 1)

Show all issues

This is the header I mean. When "hidden", this should still appear and the table body should say something like:

"This file has been hidden due to it being a minified file. [Show diff]."

reviewboard/static/rb/js/pages/views/diffViewerPageView.js (Diff revision 5)

Show all issues

We do not want to set the block to be invisible. We still want to see the header describing the file.

reviewboard/templates/diffviewer/view_diff.html (Diff revision 5)
Show all issues
```
mf is not descriptive enough a selector.
```

Why the change in design? I thought we wanted to let people expand them individually from within the file boxes.

Description:

		In order to determine if the file is a minified file, I have to change the file in the database so that the will end with .min.css, .min.js, or .min.less.

		First, I went to localhost:8080. Then I went to the top right bar, under my username I clicked into admin. Then, I went to database near the top left navigation bar, under DIFFVIEWER, I clicked into file diff and changed various file names to file.min.js and file.min.css and file.min.less on both source and destination file to test.

		Now there are some files with keywords ending with ".min.", I can test to modify files with those substring.
		-10/19

		I am trying to figure out how the diffReviewableView.js is connected with diff_file_fragment.html to link the onclick events with the button.

		I also need to know what class the contents are being wrapped around so that I can show/hide those contents when I click the button.
		-10/24

		The new Show/Hide .min button can now show and hide the diff-box. The location of the button is now updated. I added the button to where the other expand/collapse buttons were for managing data within the diff-box. I still need to only show the new button for .min.css, .min.js, and .min.less files.
		-10/31
	+
	+	I have to add a function in the python code so that it will recognize that the file name is a minified file. I'm still working on understanding the Python source code for whitespace so that I can have a similar message for the minified file. I need to hide the contents of the minified files by default, and have a link that when pressed, will show those contents.
	+	-11/6

Testing Done:

		Test string inputs to see where they are being displayed
		-10/19

		Test button to see where they are displayed. Test button functionality with alert()
		Test button functionality with diffReviewable.js
		-10/24

		Add button location with view_diff.html
		Test button functionality with diffViewerPageView.js
		Remove diff-highlight when .min file is hidden
		Change font-weight of Show/Hide .min button to normal weight
		-10/31
	+
	+	Working on understanding python source code for whitespace, no code updated yet
	+	-11/6

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js (Diff revision 5)
Show all issues
```
It looks like there is an extre newline here.
```

reviewboard/static/rb/js/pages/views/diffViewerPageView.js (Diff revision 5)

Show all issues

I think these change comments are not very helpful. All changes are already highlighted in the diff viewer and git can be used to display when a specific portion of code was added.

Description:

		In order to determine if the file is a minified file, I have to change the file in the database so that the will end with .min.css, .min.js, or .min.less.

		First, I went to localhost:8080. Then I went to the top right bar, under my username I clicked into admin. Then, I went to database near the top left navigation bar, under DIFFVIEWER, I clicked into file diff and changed various file names to file.min.js and file.min.css and file.min.less on both source and destination file to test.

		Now there are some files with keywords ending with ".min.", I can test to modify files with those substring.
		-10/19

		I am trying to figure out how the diffReviewableView.js is connected with diff_file_fragment.html to link the onclick events with the button.

		I also need to know what class the contents are being wrapped around so that I can show/hide those contents when I click the button.
		-10/24

		The new Show/Hide .min button can now show and hide the diff-box. The location of the button is now updated. I added the button to where the other expand/collapse buttons were for managing data within the diff-box. I still need to only show the new button for .min.css, .min.js, and .min.less files.
		-10/31

		I have to add a function in the python code so that it will recognize that the file name is a minified file. I'm still working on understanding the Python source code for whitespace so that I can have a similar message for the minified file. I need to hide the contents of the minified files by default, and have a link that when pressed, will show those contents.
		-11/6
	+
	+	The diff review page now shows the message to detect that the file is a minified file. I still have to change the css for the message (currently uses whitespace class). I also need to make hide the contents of the minified file by default and create a link that will show the contents of the file when pressed.
	+	-11/14

Testing Done:

		Test string inputs to see where they are being displayed
		-10/19

		Test button to see where they are displayed. Test button functionality with alert()
		Test button functionality with diffReviewable.js
		-10/24

		Add button location with view_diff.html
		Test button functionality with diffViewerPageView.js
		Remove diff-highlight when .min file is hidden
		Change font-weight of Show/Hide .min button to normal weight
		-10/31

		Working on understanding python source code for whitespace, no code updated yet
		-11/6
	+
	+	Add new member of the dictionary in diff_utils.py
	+	Add .min. file message in diff_file_fragment.html
	+	Test min-link class in diffReviewable.js
	+	-11/14

Diff:

Revision 6 (+56 -5)

Show changes

	reviewboard/diffviewer/diffutils.py
	reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
	reviewboard/static/rb/js/pages/views/diffViewerPageView.js
	reviewboard/templates/diffviewer/diff_file_fragment.html
	reviewboard/templates/diffviewer/view_diff.html

Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/diffutils.py

Ignored Files:
    reviewboard/templates/diffviewer/view_diff.html
    reviewboard/templates/diffviewer/diff_file_fragment.html
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/static/rb/js/pages/views/diffViewerPageView.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/diffutils.py

Ignored Files:
    reviewboard/templates/diffviewer/view_diff.html
    reviewboard/templates/diffviewer/diff_file_fragment.html
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/static/rb/js/pages/views/diffViewerPageView.js

Diff:

Revision 7 (+16 -4)

Show changes

	reviewboard/diffviewer/diffutils.py
	reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
	reviewboard/templates/diffviewer/diff_file_fragment.html
	reviewboard/templates/diffviewer/view_diff.html

Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/diffutils.py

Ignored Files:
    reviewboard/templates/diffviewer/view_diff.html
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/templates/diffviewer/diff_file_fragment.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/diffutils.py

Ignored Files:
    reviewboard/templates/diffviewer/view_diff.html
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/templates/diffviewer/diff_file_fragment.html

Testing Done:

		Test string inputs to see where they are being displayed
		-10/19

		Test button to see where they are displayed. Test button functionality with alert()
		Test button functionality with diffReviewable.js
		-10/24

		Add button location with view_diff.html
		Test button functionality with diffViewerPageView.js
		Remove diff-highlight when .min file is hidden
		Change font-weight of Show/Hide .min button to normal weight
		-10/31

		Working on understanding python source code for whitespace, no code updated yet
		-11/6

		Add new member of the dictionary in diff_utils.py
		Add .min. file message in diff_file_fragment.html
		Test min-link class in diffReviewable.js
	+	Remove old button and it's functions
		-11/14

Diff:

Revision 8 (+16 -4)

Show changes

	reviewboard/diffviewer/diffutils.py
	reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
	reviewboard/templates/diffviewer/diff_file_fragment.html
	reviewboard/templates/diffviewer/view_diff.html

Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/diffutils.py

Ignored Files:
    reviewboard/templates/diffviewer/view_diff.html
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/templates/diffviewer/diff_file_fragment.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/diffutils.py

Ignored Files:
    reviewboard/templates/diffviewer/view_diff.html
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js
    reviewboard/templates/diffviewer/diff_file_fragment.html

You have a pending review.

Review Board 5.0.2 alpha 0 (dev)

[WIP] Add Show/Hide .min button for minified source files

Summary:

Testing Done:

Summary:

Testing Done:

Description:

Testing Done:

Added Files:

Diff:

Diff:

Diff:

Summary:

Description:

Testing Done:

Diff:

Removed Files:

Added Files:

Description:

Testing Done:

Description:

Testing Done:

Diff:

Diff:

Testing Done:

Diff:

Status: Discarded