File Attachment Revision Slider
Review Request #6591 — Created Nov. 15, 2014 and submitted
The slider is now functional for image attachments and text attachments (including rendered text), initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js)The URL argument is what is used to specify the files being diffed. ".../r/<rrID>/<FileAttachmentID>-<DiffID>/"
The first option in the slider is the No Diff option, which will redirect to
.../r/<rrID>/<FileAttachmentID>/
This is how switching back to single files works.
This mirrors the way the slider works in review requests. (diffRevisionSelectorView.js).A title has been added to the slider that behaves the same as the title for the diffviewer revision slider. The class implementation is more or less the same except is refers to the fileAttachmentRevisionModel instead of the diffRevisionModel.
Clicking on labels other than the no diff will redirect to that single file view.
I'm having trouble changing the text attachment title header to have each file title allign with their respective file, so for now they are just spaced out with an & symbol.
TODO:
- Text File title allignment (columns?)
- CSS Spacing issues?For another Review Request
- Adding coverage to unit tests
- soft redirect
Verified by hand that url argument <ID>-<diffID> throws a 404 if the two IDs do not share attachment history for both images and text attachments.
All unit tests pass
Manually tested the slider for text, markdown and images.
| Description | From | Last Updated |
|---|---|---|
|
Why does the revision slider make the footer of the table beige? Can it be modified so that the footer … |
 brennie
|
|
|
Col: 80 E501 line too long (188 > 79 characters) |
 reviewbot
|
|
|
Should only have one var statement. |
 chipx86
|
|
|
Spaces around the -. |
chipx86
|
|
|
This would be easier to read if you only did the conditional once, and computed the proper set of values … |
chipx86
|
|
|
Here too. |
chipx86
|
|
|
I don't think the default UI should be deciding where this goes, or should even place it anywhere. It should … |
chipx86
|
|
|
Col: 80 E501 line too long (80 > 79 characters) |
reviewbot
|
|
|
We're doing this twice. Best to get the results once, convert to a list, then use it both here and … |
chipx86
|
|
|
Alphabetical order. |
chipx86
|
|
|
Blank line between these. |
chipx86
|
|
|
Indentation error on the mark_safe line. |
chipx86
|
|
|
And between these. |
chipx86
|
|
|
Blank line here. |
chipx86
|
|
|
And here. |
chipx86
|
|
|
Can you add comments on exactly what's happening here? It's unclear. |
chipx86
|
|
|
Blank line here. |
chipx86
|
|
|
We're doing two requests needlessly here, and database requests are expensive. I think we should just do a get_object_or_404 with … |
chipx86
|
|
|
Arguments should align. |
chipx86
|
|
|
This is a bit long. How about just _revisionSelectorView? |
chipx86
|
|
|
Arguments must align. |
chipx86
|
|
|
Looks like a missing ` on base. |
chipx86
|
|
|
No blank line here. |
chipx86
|
|
|
This should go before the variables without values. |
chipx86
|
|
|
Blank line here. |
chipx86
|
|
|
Multi-line comments should use the: /* * */ form. |
chipx86
|
|
|
Let's compute a URL once, then use window.location.replace(). |
chipx86
|
|
|
Same comment about the name. |
chipx86
|
|
|
Same comment about alignment. Above comments also all apply to this file as well. |
chipx86
|
- Change Summary:
-
Properly implemented the setup function so that the slider now reflects the number of revisions associated with a fileAttachment.
- Commit:
-
9454c8662a6a050f8348d1d2e4e2a2e8e38d822addae53d5970bba484a0f41f1fc56f6fbbd434d4e
- Diff:
-
Revision 2 (+35 -6)
-
Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/ui/base.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/templates/reviews/ui/default.html reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js Tool: Pyflakes Processed Files: reviewboard/reviews/ui/base.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/templates/reviews/ui/default.html reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js
- Change Summary:
-
Addressed style issues, moved html div for the slider from default.html to imageReviewableView.js
- Commit:
-
ddae53d5970bba484a0f41f1fc56f6fbbd434d4e12b61f56af8a57fb378d67e6bfefebbae6c7c0cf
-
Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/ui/base.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js Tool: Pyflakes Processed Files: reviewboard/reviews/ui/base.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js
- Change Summary:
-
WIP code for adding a column to a text file attachment template for a 2nd file
Added revisionSelected Callbacks for the slider in both image and text file file attachment views
Added a slider render to the text attachment (this one is declared in the text.html template) - Description:
-
~ Slider now renders on image view, but doesn't do anything yet.
~ Slider now renders on image and text attachment review, but and has callbacks that grab the revision number of the file history. (But don't yet do anything with them).
- Commit:
-
12b61f56af8a57fb378d67e6bfefebbae6c7c0cfc98eef35c8262f1243b369b47149668113728a57
- Diff:
-
Revision 4 (+100 -8)
-
Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/ui/base.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js Tool: Pyflakes Processed Files: reviewboard/reviews/ui/base.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js
- Change Summary:
-
Added image diff slider that uses hard redirects through URL changes.
- Description:
-
~ Slider now renders on image and text attachment review, but and has callbacks that grab the revision number of the file history. (But don't yet do anything with them).
~ The slider is now functional for image attachments, initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
+ With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js) + + There is still some undefined behaviour about how the attachment diffing should be initiated or turned off (look at 2 files or 1 basically). Right now moving the slider triggers a diff but there is no way back.
+ + Slider renders on text attachment review, but has callbacks that grab the revision number of the file history. (But don't yet do anything with them).
- Testing Done:
-
~ None yet.
~ Verified by hand that url argument <ID>-<diffID> throws a 404 if the two IDs do not share attachment history.
- Commit:
-
c98eef35c8262f1243b369b47149668113728a5794d1d0bc32f94ede47e181387d8966cc90f2de6d
- Diff:
-
Revision 5 (+129 -9)
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js
- Description:
-
The slider is now functional for image attachments, initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js) + The URL argument is what is used to specify the files being diffed. ".../r/<rrID>/<FileAttachmentID>-<DiffID>/"
+ There is still some undefined behaviour about how the attachment diffing should be initiated or turned off (look at 2 files or 1 basically). Right now moving the slider triggers a diff but there is no way back.
Slider renders on text attachment review, but has callbacks that grab the revision number of the file history. (But don't yet do anything with them).
- Added Files:
- Change Summary:
-
Added text attachment slider
- Description:
-
~ The slider is now functional for image attachments, initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
~ The slider is now functional for image attachments and text attachments, initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js) The URL argument is what is used to specify the files being diffed. ".../r/<rrID>/<FileAttachmentID>-<DiffID>/"
There is still some undefined behaviour about how the attachment diffing should be initiated or turned off (look at 2 files or 1 basically). Right now moving the slider triggers a diff but there is no way back.
~ Slider renders on text attachment review, but has callbacks that grab the revision number of the file history. (But don't yet do anything with them).
~ There is some style (CSS?) issues with the sldier falling on top of a shadow edge on the text box. I'm open to suggestions on how to resolve that.
+ + TODO:
+ Markdown text + Initial slider and attachment state when "review" pageis visited + Review state control (1 or 2 files) - Testing Done:
-
~ Verified by hand that url argument <ID>-<diffID> throws a 404 if the two IDs do not share attachment history.
~ Verified by hand that url argument <ID>-<diffID> throws a 404 if the two IDs do not share attachment history for both images and text attachments.
- Commit:
-
94d1d0bc32f94ede47e181387d8966cc90f2de6d54fb94e3da90c600ee9305db7f15a8c8718248f0
- Diff:
-
Revision 6 (+153 -15)
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/ui/text.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/ui/text.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js
- Change Summary:
-
Added markdown diffing
- Description:
-
~ The slider is now functional for image attachments and text attachments, initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
~ The slider is now functional for image attachments and text attachments (including rendered text), initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js) The URL argument is what is used to specify the files being diffed. ".../r/<rrID>/<FileAttachmentID>-<DiffID>/"
There is still some undefined behaviour about how the attachment diffing should be initiated or turned off (look at 2 files or 1 basically). Right now moving the slider triggers a diff but there is no way back.
~ There is some style (CSS?) issues with the sldier falling on top of a shadow edge on the text box. I'm open to suggestions on how to resolve that.
~ TODO:
~ TODO:
~ Initial slider and attachment state when "review" pageis visited
Review state control (1 or 2 files)
Text clider CSS Issue (I don't where to look to resolve this)
Testing?- Markdown text - Initial slider and attachment state when "review" pageis visited - Review state control (1 or 2 files) - Commit:
-
54fb94e3da90c600ee9305db7f15a8c8718248f073b86091bec297b004b30d9d8a151cef99a5edcf
- Diff:
-
Revision 7 (+181 -21)
- Added Files:
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/ui/text.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/ui/text.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js -
- Description:
-
The slider is now functional for image attachments and text attachments (including rendered text), initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js) The URL argument is what is used to specify the files being diffed. ".../r/<rrID>/<FileAttachmentID>-<DiffID>/"
There is still some undefined behaviour about how the attachment diffing should be initiated or turned off (look at 2 files or 1 basically). Right now moving the slider triggers a diff but there is no way back.
~ TODO:
~ ~ Initial slider and attachment state when "review" pageis visited
Review state control (1 or 2 files)
Text clider CSS Issue (I don't where to look to resolve this)
Testing?~ TODO:
~ - Initial slider and attachment state when "review" pageis visited ~ - Review state control (1 or 2 files) + - Testing + - Text clider CSS Issue (I don't where to look to resolve this) + - soft redirect + - Title for second file
- Change Summary:
-
Moving out of WIP
- Summary:
-
[WIP] File Attachment Revision SliderFile Attachment Revision Slider
- Description:
-
The slider is now functional for image attachments and text attachments (including rendered text), initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js) The URL argument is what is used to specify the files being diffed. ".../r/<rrID>/<FileAttachmentID>-<DiffID>/"
There is still some undefined behaviour about how the attachment diffing should be initiated or turned off (look at 2 files or 1 basically). Right now moving the slider triggers a diff but there is no way back.
TODO:
- Initial slider and attachment state when "review" pageis visited - Review state control (1 or 2 files) ~ - Testing ~ - Adding to unit tests? - Text clider CSS Issue (I don't where to look to resolve this) - soft redirect - Title for second file - Testing Done:
-
Verified by hand that url argument <ID>-<diffID> throws a 404 if the two IDs do not share attachment history for both images and text attachments.
+ + All unit tests pass
- Description:
-
The slider is now functional for image attachments and text attachments (including rendered text), initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js) The URL argument is what is used to specify the files being diffed. ".../r/<rrID>/<FileAttachmentID>-<DiffID>/"
There is still some undefined behaviour about how the attachment diffing should be initiated or turned off (look at 2 files or 1 basically). Right now moving the slider triggers a diff but there is no way back.
TODO:
~ - Initial slider and attachment state when "review" pageis visited ~ - Initial slider and attachment state when "review" page is visited - Review state control (1 or 2 files) - Adding to unit tests? - Text clider CSS Issue (I don't where to look to resolve this) - soft redirect - Title for second file
- Change Summary:
-
Changed slider behaviour to mirror the diff revision slider in review requests and slider position to above attachment.
Added new screenshots - Description:
-
The slider is now functional for image attachments and text attachments (including rendered text), initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js) + + The URL argument is what is used to specify the files being diffed. ".../r/<rrID>/<FileAttachmentID>-<DiffID>/"
~ There is still some undefined behaviour about how the attachment diffing should be initiated or turned off (look at 2 files or 1 basically). Right now moving the slider triggers a diff but there is no way back.
~ The first option in the slider is the No Diff option, which will redirect to
+ .../r/<rrID>/<FileAttachmentID>/ + This is how switching back to single files works. + This mirrors the way the slider works in review requests. (diffRevisionSelectorView.js). + + The CSS was modified to pad (left and top) the slider into position, but I was not able to remove the space below the slider in the div area (attempted margin-bottom and padding bottom to no avail). Does the slider object inately generate space below?
+ + TODO:
- - Initial slider and attachment state when "review" page is visited - - Review state control (1 or 2 files) - Adding to unit tests? - - Text clider CSS Issue (I don't where to look to resolve this) - soft redirect ~ - Title for second file ~ - Title for second file + - Slider title + - CSS Bottom Border - Commit:
-
73b86091bec297b004b30d9d8a151cef99a5edcfa718819f007550be72193313fdd3b635ba34d2d3
- Diff:
-
Revision 8 (+210 -22)
- Removed Files:
- Added Files:
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/ui/text.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/css/pages/text-review-ui.less reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/ui/text.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/css/pages/text-review-ui.less reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js
- Change Summary:
-
Addressed changes in Christians review, and updated title display for files when it diff mode (the Title now shows both file titles and captions).
- Description:
-
The slider is now functional for image attachments and text attachments (including rendered text), initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js) The URL argument is what is used to specify the files being diffed. ".../r/<rrID>/<FileAttachmentID>-<DiffID>/"
The first option in the slider is the No Diff option, which will redirect to
.../r/<rrID>/<FileAttachmentID>/ This is how switching back to single files works. This mirrors the way the slider works in review requests. (diffRevisionSelectorView.js). The CSS was modified to pad (left and top) the slider into position, but I was not able to remove the space below the slider in the div area (attempted margin-bottom and padding bottom to no avail). Does the slider object inately generate space below?
~ ~ I'm having trouble changing the text attachment title header to have each file title allign with their respective file, so for now they are just spaced out with an & symbol.
TODO:
- Adding to unit tests? - soft redirect - Title for second file - Slider title - CSS Bottom Border - Commit:
-
a718819f007550be72193313fdd3b635ba34d2d3949b750b6e29e1a384ce086916eae78ccf20b540
- Diff:
-
Revision 9 (+259 -25)
- Added Files:
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/ui/text.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/css/pages/text-review-ui.less reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/ui/text.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/css/pages/text-review-ui.less reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js
- Change Summary:
-
Added a slider title that behaves the same as the diffRevisionLabel that works with the diffRevisionSelectorView.
- Description:
-
The slider is now functional for image attachments and text attachments (including rendered text), initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js) The URL argument is what is used to specify the files being diffed. ".../r/<rrID>/<FileAttachmentID>-<DiffID>/"
The first option in the slider is the No Diff option, which will redirect to
.../r/<rrID>/<FileAttachmentID>/ This is how switching back to single files works. This mirrors the way the slider works in review requests. (diffRevisionSelectorView.js). ~ The CSS was modified to pad (left and top) the slider into position, but I was not able to remove the space below the slider in the div area (attempted margin-bottom and padding bottom to no avail). Does the slider object inately generate space below?
~ A title has been added to the slider that behaves the same as the title for the diffviewer revision slider. The class implementation is more or less the same except is refers to the fileAttachmentRevisionModel instead of the diffRevisionModel.
+ + Clicking on labels other than the no diff will redirect to that single file view.
I'm having trouble changing the text attachment title header to have each file title allign with their respective file, so for now they are just spaced out with an & symbol.
TODO:
- Adding to unit tests? - soft redirect ~ - Title for second file ~ - Slider title ~ - Text File title allignment (columns?) ~ - CSS Spacing issues? - - CSS Bottom Border - Commit:
-
949b750b6e29e1a384ce086916eae78ccf20b540884ccbac1acf48ce0fbea8535e9e8e662418a90f
- Diff:
-
Revision 10 (+369 -25)
- Added Files:
-
Tool: Pyflakes Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/ui/text.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/css/pages/text-review-ui.less reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js reviewboard/static/rb/js/views/fileAttachmentRevisionLabelView.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/views.py reviewboard/reviews/ui/base.py reviewboard/reviews/ui/text.py reviewboard/reviews/urls.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/css/pages/text-review-ui.less reviewboard/templates/reviews/ui/text.html reviewboard/static/rb/js/views/textBasedReviewableView.js reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js reviewboard/static/rb/js/views/fileAttachmentRevisionLabelView.js
- Description:
-
The slider is now functional for image attachments and text attachments (including rendered text), initiating the controls for different diff modes. The slider actions trigger a hard redirect for now (window.location.replace).
With enough time I may be able to do a soft redirect using router and ajax (like _loadRevision in diffViewerPageView.js) The URL argument is what is used to specify the files being diffed. ".../r/<rrID>/<FileAttachmentID>-<DiffID>/"
The first option in the slider is the No Diff option, which will redirect to
.../r/<rrID>/<FileAttachmentID>/ This is how switching back to single files works. This mirrors the way the slider works in review requests. (diffRevisionSelectorView.js). A title has been added to the slider that behaves the same as the title for the diffviewer revision slider. The class implementation is more or less the same except is refers to the fileAttachmentRevisionModel instead of the diffRevisionModel.
Clicking on labels other than the no diff will redirect to that single file view.
I'm having trouble changing the text attachment title header to have each file title allign with their respective file, so for now they are just spaced out with an & symbol.
TODO:
- - Adding to unit tests? - - soft redirect - Text File title allignment (columns?) - CSS Spacing issues? + + For another Review Request
+ - Adding coverage to unit tests + - soft redirect - Testing Done:
-
Verified by hand that url argument <ID>-<diffID> throws a 404 if the two IDs do not share attachment history for both images and text attachments.
All unit tests pass
+ + Manually tested the slider for text, markdown and images.
Tool: Pyflakes Processed Files: reviewboard/reviews/ui/base.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/templates/reviews/ui/default.html reviewboard/static/rb/js/views/revisionSelectorView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/ui/base.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/fileAttachmentReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.js reviewboard/templates/reviews/ui/default.html reviewboard/static/rb/js/views/revisionSelectorView.js reviewboard/static/rb/js/views/fileAttachmentRevisionSelectorView.js