Add inline help text for hosting services in Admin ADD REPOSITORY page.

Review Request #10890 — Created Feb. 6, 2020 and updated

xiaole2
Review Board
master
10872
reviewboard

Create docs_url and summary attributes in class HostingService and SCMTools.

Fill details in each hosting services (not include Kiln On Demand and Review Board Gateway, because right now we don't have references provided by ReviewBoard Manual.

  1. Hosting Servive: $hostingHelp is a new paragraph with class="hosting-help-text" in the template for the RepositoryForm javascript file. By default, $hostingHelp will nonvisiable. When users select a hosting service, the inline text will show up.

  2. SCMTools: $repoHelp is a new paragraph with class="repo-help-text" in the template for the RepositoryForm javascript file. Only when a user click 'custom' as the hosting service, $repoHelp will visiable. When users select a repo type, the inline text will show up.

Tested in Google Chrome, Firefox and Safari. Inline Text of each hosting service displays correctly in the page.
Tested ./tests/runtests.py reviewboard.webapi.tests.test_hosting_service successfully.

Summary Author
Introduce attributes into HostingService and fill them in each hosting service
XiaoleZ
modify the coding format, remove trailing space
XiaoleZ
Create hosting-inline-help-text attribute in HostingService, and fill details in each hosting services. Update template for the RepositoryForm file
XiaoleZ
Remove extra () for import, and remove InlineHelpTextBase Class(wrong direction)
XiaoleZ
split inline_help_text into two attributes, change toggle() into hide() and show()
XiaoleZ
updatethe formate
XiaoleZ
remove trailing space for repositoryform.es6.js
XiaoleZ
remove unicode(), replace html() with text(), add tag <a>
XiaoleZ
clean code
XiaoleZ
change repositoryform.js
XiaoleZ
remove trailing space
XiaoleZ
add ;
XiaoleZ
implementating scmtools
XiaoleZ
rebase from the master/setup repoHelp show() condition
XiaoleZ
fix line too long
XiaoleZ
Loading file attachments...

Description From Last Updated

Good start! I still want to see this as two attributes: summary and docs_url. The form should be able to ...

chipx86chipx86

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (111 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (113 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (113 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (116 > 79 characters)

reviewbotreviewbot

E501 line too long (123 > 79 characters)

reviewbotreviewbot

E501 line too long (127 > 79 characters)

reviewbotreviewbot

E501 line too long (129 > 79 characters)

reviewbotreviewbot

E501 line too long (127 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (120 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (107 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (107 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (107 > 79 characters)

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (113 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (117 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (117 > 79 characters)

reviewbotreviewbot

These will need to all be localized using ugettext_lazy() (often aliased as _() -- see the imports in each file).

chipx86chipx86

It's important not to hard-code the full path to the manual. Use reviewboard.get_manual_url() as the base of the URL instead. ...

chipx86chipx86

.toggle() isn't safe to use. You want an explicit hide() or show().

chipx86chipx86

E225 missing whitespace around operator

reviewbotreviewbot

E225 missing whitespace around operator

reviewbotreviewbot

E225 missing whitespace around operator

reviewbotreviewbot

E225 missing whitespace around operator

reviewbotreviewbot

E225 missing whitespace around operator

reviewbotreviewbot

E225 missing whitespace around operator

reviewbotreviewbot

E225 missing whitespace around operator

reviewbotreviewbot

E225 missing whitespace around operator

reviewbotreviewbot

Col: 40 Missing semicolon.

reviewbotreviewbot

Col: 76 Missing semicolon.

reviewbotreviewbot

Col: 81 Missing semicolon.

reviewbotreviewbot

Anything inside a _(...) must be a pure string literal, no string formatting allowed. This is because the localization text ...

chipx86chipx86

You'll just want to use _(...), not unicode(_(...)). In fact, this is a syntax error on Python 3 (unicode is ...

chipx86chipx86

We're not going to want HTML in this. We might be rendering to HTML in the case of this form, ...

chipx86chipx86

I don't think we need this variable. The code for the form should be responsible for determining how we're representating/linking ...

chipx86chipx86

You'll want to use %s instead of + for building strings.

chipx86chipx86

Col: 34 Missing semicolon.

reviewbotreviewbot

Col: 26 Missing semicolon.

reviewbotreviewbot

E501 line too long (93 > 79 characters)

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

xiaole2
xiaole2
xiaole2
xiaole2
xiaole2
chipx86
  1. 
      
  2. Good start!

    I still want to see this as two attributes: summary and docs_url. The form should be able to handle either, both, or none of those values, and should have a standard link (probably as a paragraph following the summary) for linking to the documentation.

  3. reviewboard/hostingsvcs/assembla.py (Diff revision 4)
     
     
     
     
     
     
     
     
     
     
     

    These will need to all be localized using ugettext_lazy() (often aliased as _() -- see the imports in each file).

  4. reviewboard/hostingsvcs/assembla.py (Diff revision 4)
     
     
     

    It's important not to hard-code the full path to the manual. Use reviewboard.get_manual_url() as the base of the URL instead. See reviewboard/__init__.py for the definition of that function.

  5. .toggle() isn't safe to use. You want an explicit hide() or show().

  6. 
      
xiaole2
Review request changed

Change Summary:

  1. Splite inline_help_text into two attributes(summary + support_docs).
  2. Set docs_url as another attribute (use from reviewboard import get_manual_url())
  3. Change toggle() into hide() and show()
  4. Use ugettext_lazy() as _() for text

Commits:

Summary Author
-
Introduce attributes into HostingService and fill them in each hosting service
XiaoleZ
-
modify the coding format, remove trailing space
XiaoleZ
-
Create hosting-inline-help-text attribute in HostingService, and fill details in each hosting services. Update template for the RepositoryForm file
XiaoleZ
-
Remove extra () for import, and remove InlineHelpTextBase Class(wrong direction)
XiaoleZ
+
Introduce attributes into HostingService and fill them in each hosting service
XiaoleZ
+
modify the coding format, remove trailing space
XiaoleZ
+
Create hosting-inline-help-text attribute in HostingService, and fill details in each hosting services. Update template for the RepositoryForm file
XiaoleZ
+
Remove extra () for import, and remove InlineHelpTextBase Class(wrong direction)
XiaoleZ
+
split inline_help_text into two attributes, change toggle() into hide() and show()
XiaoleZ

Diff:

Revision 5 (+1035 -431)

Show changes

Checks run (2 failed)

flake8 failed.
JSHint failed.

flake8

JSHint

xiaole2
xiaole2
chipx86
  1. 
      
  2. reviewboard/hostingsvcs/assembla.py (Diff revision 7)
     
     
     
     
     

    Anything inside a _(...) must be a pure string literal, no string formatting allowed. This is because the localization text scanner is going to look for strings inside of _(...) and include those directly. So we can't ever use % ... here.

    In the case of a URL, there's nothing to localize anyway, so we don't need to use _(...), but the above applies to the other strings.

  3. reviewboard/hostingsvcs/assembla.py (Diff revision 7)
     
     

    You'll just want to use _(...), not unicode(_(...)).

    In fact, this is a syntax error on Python 3 (unicode is not a usable keyword).

  4. reviewboard/hostingsvcs/assembla.py (Diff revision 7)
     
     
     
     

    We're not going to want HTML in this. We might be rendering to HTML in the case of this form, but there's no guarantee that we'll be using this for HTML. For instance, we'll want to provide this in the API, which isn't HTML-based.

    We also don't need to use %s here for the name. Just including "Assembla" verbatim is fine.

    1. I wonder what API we are using? how to add it in the API? does the file means the data already in the API?
      We want to the plain text for the summary. And in the RepositoryForm.js we render this text in HTML-based?

    2. Review Board exposes an API that's used by a number of things, including Review Board itself, third-party clients, automation scripts, etc. This all lives in reviewboard/webapi/. As part of this project, in an upcoming step, I'm going to have you add a field in the API to show the summary (this would live in reviewboard/webapi/resources/hosting_service.py). This would allow clients of the API to list all hosting services and see their summaries (alongside the information we already provide).

      The JavaScript for the repository form will render this text. It will treat it as plain text, not HTML (so, you'd use .text(...) to set it on an element instead of .html(...)).

    3. Oh, and you may want to see the documentation (and example payloads) for the API, to get a sense of what kind of stuff we expose.

  5. reviewboard/hostingsvcs/assembla.py (Diff revision 7)
     
     
     
     
     

    I don't think we need this variable. The code for the form should be responsible for determining how we're representating/linking to the URL, not this class.

    1. So I need to determine how to link the url in repositoryform.js?
      If it has docs_url, then render the information as About %s repository in Review Board

    2. I would just call the link something like "Learn More", but yes. repositoryform.js would be responsible for building the <a> tag for this, plugging in the docs URL.

  6. reviewboard/hostingsvcs/assembla.py (Diff revision 7)
     
     
     

    You'll want to use %s instead of + for building strings.

  7. 
      
xiaole2
Review request changed

Change Summary:

remove unicode(), replace html() with text(), add tag <a>

Description:

~  

Create hosting_inline_help_text attribute in class HostingService.

  ~

Create docs_url and summary attributes in class HostingService.

   
   

Fill details in each hosting services (not include Kiln On Demand and Review Board Gateway, because right now we don't have references provided by ReviewBoard Manual.

   
   

$inlineHelpText is a new paragraph with class="hosting_inline_help_text" in the template for the RepositoryForm javascript file. By default, $inlineHelpText will nonvisiable. When users select a hosting service, the inline text will show up.

Testing Done:

~  

Tested in Google Chrome and Firefox. Inline Text of each hosting service displays correctly in the page.

  ~

Tested in Google Chrome, Firefox and Safari. Inline Text of each hosting service displays correctly in the page.

  + Tested ./tests/runtests.py reviewboard.webapi.tests.test_hosting_service successfully.

Commits:

Summary Author
-
Introduce attributes into HostingService and fill them in each hosting service
XiaoleZ
-
modify the coding format, remove trailing space
XiaoleZ
-
Create hosting-inline-help-text attribute in HostingService, and fill details in each hosting services. Update template for the RepositoryForm file
XiaoleZ
-
Remove extra () for import, and remove InlineHelpTextBase Class(wrong direction)
XiaoleZ
-
split inline_help_text into two attributes, change toggle() into hide() and show()
XiaoleZ
-
updatethe formate
XiaoleZ
-
remove trailing space for repositoryform.es6.js
XiaoleZ
+
Introduce attributes into HostingService and fill them in each hosting service
XiaoleZ
+
modify the coding format, remove trailing space
XiaoleZ
+
Create hosting-inline-help-text attribute in HostingService, and fill details in each hosting services. Update template for the RepositoryForm file
XiaoleZ
+
Remove extra () for import, and remove InlineHelpTextBase Class(wrong direction)
XiaoleZ
+
split inline_help_text into two attributes, change toggle() into hide() and show()
XiaoleZ
+
updatethe formate
XiaoleZ
+
remove trailing space for repositoryform.es6.js
XiaoleZ
+
remove unicode(), replace html() with text(), add tag <a>
XiaoleZ
+
clean code
XiaoleZ
+
change repositoryform.js
XiaoleZ
+
remove trailing space
XiaoleZ

Diff:

Revision 8 (+1066 -640)

Show changes

Removed Files:

Added Files:

Checks run (1 failed, 1 succeeded)

flake8 passed.
JSHint failed.

JSHint

xiaole2
xiaole2
Review request changed

Change Summary:

Add inline help for SCMTools

Description:

~  

Create docs_url and summary attributes in class HostingService.

  ~

Create docs_url and summary attributes in class HostingService and SCMTools.

   
   

Fill details in each hosting services (not include Kiln On Demand and Review Board Gateway, because right now we don't have references provided by ReviewBoard Manual.

   
~  

$inlineHelpText is a new paragraph with class="hosting_inline_help_text" in the template for the RepositoryForm javascript file. By default, $inlineHelpText will nonvisiable. When users select a hosting service, the inline text will show up.

  ~
  1. Hosting Servive: $hostingHelp is a new paragraph with class="hosting-help-text" in the template for the RepositoryForm javascript file. By default, $hostingHelp will nonvisiable. When users select a hosting service, the inline text will show up.

  +
  1. SCMTools: $repoHelp is a new paragraph with class="repo-help-text" in the template for the RepositoryForm javascript file. Only when a user click 'custom' as the hosting service, $repoHelp will visiable. When users select a repo type, the inline text will show up.

Commits:

Summary Author
-
Introduce attributes into HostingService and fill them in each hosting service
XiaoleZ
-
modify the coding format, remove trailing space
XiaoleZ
-
Create hosting-inline-help-text attribute in HostingService, and fill details in each hosting services. Update template for the RepositoryForm file
XiaoleZ
-
Remove extra () for import, and remove InlineHelpTextBase Class(wrong direction)
XiaoleZ
-
split inline_help_text into two attributes, change toggle() into hide() and show()
XiaoleZ
-
updatethe formate
XiaoleZ
-
remove trailing space for repositoryform.es6.js
XiaoleZ
-
remove unicode(), replace html() with text(), add tag <a>
XiaoleZ
-
clean code
XiaoleZ
-
change repositoryform.js
XiaoleZ
-
remove trailing space
XiaoleZ
-
add ;
XiaoleZ
+
Introduce attributes into HostingService and fill them in each hosting service
XiaoleZ
+
modify the coding format, remove trailing space
XiaoleZ
+
Create hosting-inline-help-text attribute in HostingService, and fill details in each hosting services. Update template for the RepositoryForm file
XiaoleZ
+
Remove extra () for import, and remove InlineHelpTextBase Class(wrong direction)
XiaoleZ
+
split inline_help_text into two attributes, change toggle() into hide() and show()
XiaoleZ
+
updatethe formate
XiaoleZ
+
remove trailing space for repositoryform.es6.js
XiaoleZ
+
remove unicode(), replace html() with text(), add tag <a>
XiaoleZ
+
clean code
XiaoleZ
+
change repositoryform.js
XiaoleZ
+
remove trailing space
XiaoleZ
+
add ;
XiaoleZ
+
implementating scmtools
XiaoleZ
+
rebase from the master/setup repoHelp show() condition
XiaoleZ

Diff:

Revision 10 (+1295 -663)

Show changes

Removed Files:

Added Files:

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

xiaole2
Review request changed

Change Summary:

fix line too long

Commits:

Summary Author
-
Introduce attributes into HostingService and fill them in each hosting service
XiaoleZ
-
modify the coding format, remove trailing space
XiaoleZ
-
Create hosting-inline-help-text attribute in HostingService, and fill details in each hosting services. Update template for the RepositoryForm file
XiaoleZ
-
Remove extra () for import, and remove InlineHelpTextBase Class(wrong direction)
XiaoleZ
-
split inline_help_text into two attributes, change toggle() into hide() and show()
XiaoleZ
-
updatethe formate
XiaoleZ
-
remove trailing space for repositoryform.es6.js
XiaoleZ
-
remove unicode(), replace html() with text(), add tag <a>
XiaoleZ
-
clean code
XiaoleZ
-
change repositoryform.js
XiaoleZ
-
remove trailing space
XiaoleZ
-
add ;
XiaoleZ
-
implementating scmtools
XiaoleZ
-
rebase from the master/setup repoHelp show() condition
XiaoleZ
+
Introduce attributes into HostingService and fill them in each hosting service
XiaoleZ
+
modify the coding format, remove trailing space
XiaoleZ
+
Create hosting-inline-help-text attribute in HostingService, and fill details in each hosting services. Update template for the RepositoryForm file
XiaoleZ
+
Remove extra () for import, and remove InlineHelpTextBase Class(wrong direction)
XiaoleZ
+
split inline_help_text into two attributes, change toggle() into hide() and show()
XiaoleZ
+
updatethe formate
XiaoleZ
+
remove trailing space for repositoryform.es6.js
XiaoleZ
+
remove unicode(), replace html() with text(), add tag <a>
XiaoleZ
+
clean code
XiaoleZ
+
change repositoryform.js
XiaoleZ
+
remove trailing space
XiaoleZ
+
add ;
XiaoleZ
+
implementating scmtools
XiaoleZ
+
rebase from the master/setup repoHelp show() condition
XiaoleZ
+
fix line too long
XiaoleZ

Diff:

Revision 11 (+1297 -665)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...