-
-
This is not the fix we were hoping for. We like the minimum height when the fields are opened.
What we want instead is to change the CSS to ensure we have the right cursor.
-
Additionally, once you fix that, the summary and description need a lot of love. See https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/ for an explanation of what we look for in these, along with examples.
Fixed text fields that have the wrong cursor displayed when hovered over by the cursor
Review Request #8403 — Created Sept. 17, 2016 and submitted
The cursor of some text fields in an edit state displayed the standard
arrow instead of the text cursor when the cursor hovered over the text
fields. Fixed text fields by setting the cursor style to text in the
Code Mirror css to ensure we have the right cursor being displayed to
the user.
- Opened the review dialog
- Added a review header
- Hovered over non-text lines
- Ensure standard arrow is not diplayed on hover
- Also, in the review Dialog, Clicked on "Add Comment"
- Added a single line comment
- Hovered over non-text lines
- Ensure standard arrow is not diplayed on hover
| Description | From | Last Updated |
|---|---|---|
|
Can you wrap your description and testing done at 72 chars? |
 brennie
|
|
|
Can you try to limit your summary to 50 chars? (Git doesnt like it when it goes over.) |
brennie
|
|
|
"non-text" in your summary Your summary should also be in the imperitive mood, i.e., "Fix text fields..." |
brennie
|
|
|
This is not the fix we were hoping for. We like the minimum height when the fields are opened. What … |
 david
|
|
|
'formataddr' imported but unused |
 reviewbot
|
|
|
'django_reset' imported but unused |
reviewbot
|
|
|
'from settings_local import *' used; unable to detect undefined names |
reviewbot
|
|
|
This file comes from a third party library, and we probably shouldn't make changes to it (since we might drop … |
david
|
-
Something that might be making this confusing to diagnose is that there's a regression in the current
release-3.0.xbranch where the last text field on the review request page isn't taking up the correct height (it's supposed to fill the available height). Most text fields won't show this issue.What you can do is go to the review dialog and add a review text header. Add a few words on one line. Note how the mouse cursor, when hovering over the first line, has a standard "I" cursor, but when hovering on the following lines (where there's not yet any text), the cursor is an arrow.
What you should be doing in this change is ensuring that the appropriate element in the text field (which uses CodeMirror, a text editor component) has the correct cursor type (
text) by default. You'll have to investigate the elements that comprise the field and try to find the most suitable parent element for this. You won't want to guess, but rather try to determine what each element means and is for, and put the CSS rule in the correct place for it.
LA
- Change Summary:
-
Added changes to css for CodeMirror text field.
Please Note that I updated my branch with master and therefore that resulted in multiple code diff
My change only pertains to the file reviewboard/static/lib/css/codemirror.css - Summary:
-
fixing error of text fields in edit states having the wrong cursor when hovering over non-text lines by setting default minimum height of text field to 0Text fields in edit states have the wrong cursor when hovering over non-text lines
- Description:
-
~ fixing error of text fields in edit states having the wrong cursor when hovering over non-text lines by setting default minimum height of text field to 0
~ The cursor of some text fields in an edit state displayed the standard arrow instead of the text cursor when the cursor hovered over non-text lines.
+ + This change sets the cursor style to text in the Code Mirror css to ensure we have the right cursor being displayed to the user when hovering over the non-text lines.
- Testing Done:
-
~ Checked text fields to ensure non-text lines do not appear. And text fields have a single line default entry.
~ - Opened the review dialog
+ - Added a review header
+ - Hovered over non-text lines
+ - Ensure standard arrow is not diplayed on hover
+ - Also, in the review Dialog, Clicked on "Add Comment"
+ - Added a single line comment
+ - Hovered over non-text lines
+ - Ensure standard arrow is not diplayed on hover
- Commit:
-
0bd789df234f1519d521726a9163db39be38edb6b660a61be9a7f9fbb15951e2df5f63e1e2428ac0
- Diff:
-
Revision 2 (+908 -284)
-
Tool: Pyflakes Processed Files: reviewboard/testing/testcase.py reviewboard/reviews/ui/image.py reviewboard/notifications/tests.py reviewboard/extensions/tests.py reviewboard/reviews/ui/tests.py reviewboard/accounts/trophies.py reviewboard/diffviewer/tests.py reviewboard/diffviewer/diffutils.py reviewboard/settings.py reviewboard/notifications/email.py reviewboard/staticbundles.py reviewboard/accounts/forms/auth.py Ignored Files: reviewboard/static/rb/css/pages/image-review-ui.less reviewboard/static/rb/js/views/regionCommentBlockView.es6.js reviewboard/static/rb/js/models/imageReviewableModel.es6.js reviewboard/static/lib/css/codemirror.css reviewboard/static/rb/css/pages/review-request.less reviewboard/static/rb/js/models/imageReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.es6.js Tool: PEP8 Style Checker Processed Files: reviewboard/testing/testcase.py reviewboard/reviews/ui/image.py reviewboard/notifications/tests.py reviewboard/extensions/tests.py reviewboard/reviews/ui/tests.py reviewboard/accounts/trophies.py reviewboard/diffviewer/tests.py reviewboard/diffviewer/diffutils.py reviewboard/settings.py reviewboard/notifications/email.py reviewboard/staticbundles.py reviewboard/accounts/forms/auth.py Ignored Files: reviewboard/static/rb/css/pages/image-review-ui.less reviewboard/static/rb/js/views/regionCommentBlockView.es6.js reviewboard/static/rb/js/models/imageReviewableModel.es6.js reviewboard/static/lib/css/codemirror.css reviewboard/static/rb/css/pages/review-request.less reviewboard/static/rb/js/models/imageReviewableModel.js reviewboard/static/rb/js/views/imageReviewableView.es6.js -
-
-
-
You'll need to make sure your branch is cleanly rebased onto master. Don't pull master into your branch. Rather, do:
$ git checkout master $ git pull origin master $ git checkout your-feature-branch $ git rebase masterIf you pulled master into your branch, you might have some issues cleaning some of that up. If so, come to us in Slack for help.
LA
- Change Summary:
-
Reverted merge and did a rebase on master
- Commit:
-
b660a61be9a7f9fbb15951e2df5f63e1e2428ac010e429add952e0ac250b92f2b641f05a24df6dfe
-
Tool: Pyflakes Ignored Files: reviewboard/static/lib/css/codemirror.css Tool: PEP8 Style Checker Ignored Files: reviewboard/static/lib/css/codemirror.css
LA
- Summary:
-
Text fields in edit states have the wrong cursor when hovering over non-text linesText fields having wrong cursor over nontext line
- Description:
-
~ The cursor of some text fields in an edit state displayed the standard arrow instead of the text cursor when the cursor hovered over non-text lines.
~ This change sets the cursor style to text in the Code Mirror css.
- - This change sets the cursor style to text in the Code Mirror css to ensure we have the right cursor being displayed to the user when hovering over the non-text lines.
- Testing Done:
-
~ - Opened the review dialog
~ - Added a review header
~ - Hovered over non-text lines
~ - Added a review header
~ - Hovered over non-text lines
~ - Ensure standard arrow is not diplayed on hover
- - Ensure standard arrow is not diplayed on hover
- - Also, in the review Dialog, Clicked on "Add Comment"
- - Added a single line comment
- - Hovered over non-text lines
- - Ensure standard arrow is not diplayed on hover
-
The code looks okay I believe (though there might be a more specific CSS selector that should be used for this), but the Description should be able to tell a story about what the problem is, so that a person doesn't have to go to the bug report or review request or dig into code.
I like to follow a "What was wrong, why, and what did we do about it" form. We have a guide and examples here: https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/
LA
- Description:
-
~ This change sets the cursor style to text in the Code Mirror css.
~ The cursor of some text fields in an edit state
+ displayed the standard arrow instead of the text + cursor when the cursor hovered over non-text lines. + This change sets the cursor style to text in the + Code Mirror css to ensure we have the right cursor + being displayed to the user when hovering over the + non-text lines. - Testing Done:
-
~ - Added a review header
~ - Hovered over non-text lines
~ - Ensure standard arrow is not diplayed on hover
~ - Opened the review dialog
~ - Added a review header
~ - Hovered over non-text lines
+ - Ensure standard arrow is not diplayed on hover
+ - Also, in the review Dialog, Clicked on "Add Comment"
+ - Added a single line comment
+ - Hovered over non-text lines
+ - Ensure standard arrow is not diplayed on hover
LA
- Description:
-
~ The cursor of some text fields in an edit state
~ displayed the standard arrow instead of the text ~ cursor when the cursor hovered over non-text lines. ~ This change sets the cursor style to text in the ~ Code Mirror css to ensure we have the right cursor ~ The cursor of some text fields in an edit state displayed the standard
~ arrow instead of the text cursor when the cursor hovered over ~ non-text lines.This change sets the cursor style to text in the Code ~ Mirror css to ensure we have the right cursor being displayed to the ~ user when hovering over the non-text lines. - being displayed to the user when hovering over the - non-text lines.
LA
- Description:
-
The cursor of some text fields in an edit state displayed the standard
arrow instead of the text cursor when the cursor hovered over ~ non-text lines.This change sets the cursor style to text in the Code ~ non-text lines. This change sets the cursor style to text in the Code Mirror css to ensure we have the right cursor being displayed to the user when hovering over the non-text lines.
-
-
This file comes from a third party library, and we probably shouldn't make changes to it (since we might drop in a replacement when we update the version of CodeMirror that we use).
We have some style overrides for
.CodeMirrorin reviewboard/static/rb/css/pages/reviews.less. Can you make this change there, instead?
-
Tool: Pyflakes Ignored Files: reviewboard/static/rb/css/pages/reviews.less Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/css/pages/reviews.less
LA
- Description:
-
The cursor of some text fields in an edit state displayed the standard
~ arrow instead of the text cursor when the cursor hovered over ~ non-text lines. This change sets the cursor style to text in the Code ~ Mirror css to ensure we have the right cursor being displayed to the ~ user when hovering over the non-text lines. ~ arrow instead of the text cursor when the cursor hovered over the text ~ fields. Fixed text fields by setting the cursor style to text in the ~ Code Mirror css to ensure we have the right cursor being displayed to ~ the user.