Summary

[WIP] Added a floating banner to the diff view

Review Request #8740 — Created Feb. 11, 2017 and discarded Feb. 16, 2017, 10:31 a.m.

Information

Owner

Mons

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard, students

People

Description

Previous State:

- The banner used to get added once per diff file. Thank you Simon Zhang 

for pointing out this bug. 

- The floating banner used to show even though the user was at the top 

of the page.

- The banner used to be bright green with a dark green border color.

- The banner contained the list of diff files only.

- The diff details inside the banner previously had its own styling class.

- The DiffReviewableView gets added to the DiffFileIndexView, which 

makes it tricky to keep track of each diff file for two different views. 

Since we are also adding a new diffFileIndexView to the FloatingBannerView

to display each diff file in the banner, we need to create a model that 

contains the state on what diffs are there or are loading.
Features Added:

- I changed the banner so that it only gets added once to the diff view, 

instead of once per diff file.

- I redesigned the styling so that the banner properly hides when the user 

is at the top.

- I added a white box that contains the diff file.

- I added some divs to the diff view that will be necessary in the future.

- I changed the banner color to match the review request body. The banner 

is now egg white with a black border color.

- I added a dropdown button on the right side of the banner (it currently 

lacks functionality).

- I removed this custom made styling class and reused the classes that have 

already been setup for this in diffviewer.less.

- I added a new DiffFileIndexModel class, but it is currently not connected 

to the views. This model class is still a work in progress.
Link to Demo Video:

https://drive.google.com/open?id=0B9Ti0bnAd2SuSmFvTWxudHFDdlU

Testing Done

I tested the banner on the testing server. I have currently only tested the
banner for review requests containing one diff file.

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/templates/diffviewer/view_diff.html
    reviewboard/static/rb/js/pages/views/diffViewerPageView.js
    reviewboard/static/rb/js/diffviewer/models/diffFileIndexModel.js
    reviewboard/static/rb/js/views/floatingBannerView.js
    reviewboard/static/rb/css/pages/diffviewer.less



Tool: Pyflakes
Ignored Files:
    reviewboard/templates/diffviewer/view_diff.html
    reviewboard/static/rb/js/pages/views/diffViewerPageView.js
    reviewboard/static/rb/js/diffviewer/models/diffFileIndexModel.js
    reviewboard/static/rb/js/views/floatingBannerView.js
    reviewboard/static/rb/css/pages/diffviewer.less

Description:: Previous State:
- The banner used to get added once per diff file. Thank you Simon Zhang
for pointing out this bug.
- The floating banner used to show even though the user was at the top
of the page.
- The banner used to be bright green with a dark green border color.
- The banner contained the list of diff files only.
- The diff details inside the banner previously had its own styling class.
- The DiffReviewableView gets added to the DiffFileIndexView, which
makes it tricky to keep track of each diff file for two different views.
Since we are also adding a new diffFileIndexView to the FloatingBannerView
to display each diff file in the banner, we need to create a model that
contains the state on what diffs are there or are loading.

Features Added:
- I changed the banner so that it only gets added once to the diff view,
instead of once per diff file.
- I redesigned the styling so that the banner properly hides when the user
is at the top.
- I added a white box that contains the diff file.
- I added some divs to the diff view that will be necessary in the future.
- I changed the banner color to match the review request body. The banner
is now egg white with a black border color.
- I added a dropdown button on the right side of the banner (it currently
lacks functionality).
- I removed this custom made styling class and reused the classes that have
already been setup for this in diffviewer.less.
- I added a new DiffFileIndexModel class, but it is currently not connected
to the views. This model class is still a work in progress.
+
+
Link to Demo Video:
+ https://drive.google.com/open?id=0B9Ti0bnAd2SuSmFvTWxudHFDdlU

Status:: Discarded
Change Summary:: multiple review requests for the same changes.

		Previous State:
		- The banner used to get added once per diff file. Thank you Simon Zhang
		for pointing out this bug.
		- The floating banner used to show even though the user was at the top
		of the page.
		- The banner used to be bright green with a dark green border color.
		- The banner contained the list of diff files only.
		- The diff details inside the banner previously had its own styling class.
		- The `DiffReviewableView` gets added to the `DiffFileIndexView`, which
		makes it tricky to keep track of each diff file for two different views.
		Since we are also adding a new `diffFileIndexView` to the `FloatingBannerView`
		to display each diff file in the banner, we need to create a model that
		contains the state on what diffs are there or are loading.

		Features Added:
		- I changed the banner so that it only gets added once to the diff view,
		instead of once per diff file.
		- I redesigned the styling so that the banner properly hides when the user
		is at the top.
		- I added a white box that contains the diff file.
		- I added some divs to the diff view that will be necessary in the future.
		- I changed the banner color to match the review request body. The banner
		is now egg white with a black border color.
		- I added a dropdown button on the right side of the banner (it currently
		lacks functionality).
		- I removed this custom made styling class and reused the classes that have
		already been setup for this in `diffviewer.less`.
		- I added a new `DiffFileIndexModel` class, but it is currently not connected
		to the views. This model class is still a work in progress.
	+
	+	Link to Demo Video:
	+	https://drive.google.com/open?id=0B9Ti0bnAd2SuSmFvTWxudHFDdlU