Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions roundup/cgi/templating.py
Original file line number Diff line number Diff line change
Expand Up @@ -3442,15 +3442,32 @@ def base_javascript(self):
return """
<script nonce="%s" type="text/javascript">
submitted = false;
function submit_once() {
submitted_restore = "";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might I suggest a different way to do this. Your current code gives the submitted variable two different meanings:

  1. a true/false flag if the button has submitted a request.
  2. the button DOM object.

While that is hacky (so lives up to the title 9-)), it ook me a bit to figure out what the (type(submitted) == type(true) was doing.

I suggest keeping submitted as a simple boolean value.

Since you added the variable: submitted_restore, make it have one of two values:

  • an unset value of null (which can be tested with submitted_restore !== null (which should test type as well as value)
  • a plain object set (at line 3465 in this diff) using:
    submitted_restore = { "target": e, "text": e.textContent, "value": e.value }
    

Doing it this way, you restore the value if it is not the same as the text string. This happens in the SubmitSilentChange example.


function submit_reset() {
$("#submit_overlay").hide();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this $(...) jquery? If so please replace it with vanilla javascript.

Going forward I hope to remove jquery entirely from all templates (issue2551423 as it is a maintainance issue: https://issues.roundup-tracker.org/issue2551422 https://issues.roundup-tracker.org/issue2551424. So I really don't want to add more.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and no problem! (the original commit used vanilla JS, I just noticed that jQuery was used and firgured that was more the direction you were using)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jquery has some specific uses for some interactions and was added back
in 2009 when IE was a thing. At that point I don't think querySelector() existed.

if (typeof(submitted)!=typeof(true)) {
submitted.value = submitted.textContent = submitted_restore;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See: https://github.com/roundup-tracker/roundup/pull/135/changes#diff-c0f24b87223e22a82a98c3206cd1d037b4b0fd5aef85706ed53bb6b9a59eeb96R3445

value and text content might not be the same specially if the text content is multiple words.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our implementation uses href= links for some submits, this was a way to cover both buttons/inputs and anchors - I did not see a conflict in my testing when both were modified but I can instead put a check in for html element vs form element to use value or text accordingly

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the href link id executing a (long running) read only operation (like a search)?

I just want to make sure that your link is not updating the database using a GET (rather than POST) method (which was something that was done with earlier Roundup implementations and does have one remnant in current Roundup).

submitted.disabled = false;
}
submitted = false;
}

function submit_once(e) {
if (submitted) {
alert("Your request is being processed.\\nPlease be patient.");
return false;
}

window.setTimeout("submitted = false;", 5000)
window.setTimeout(submit_reset, 5000);

submitted = true;
if (e) {
submitted = e;
submitted_restore = e.value ? e.value : e.textContent;
e.value = e.textContent = "Submitting...";
e.disabled = true;
} else submitted = true;
$("#submit_overlay").show();
return true;
}

Expand Down Expand Up @@ -3782,7 +3799,7 @@ def embed_form_fields(self, excluded_fields=None):

const transfer = new DataTransfer();
file_list = document.querySelectorAll('pre[data-mimetype]');
file_list.forEach( file =>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is fine since it is in the Python file that is not meant to be customized on a per tracker basis.

file_list.forEach( file =>
transfer.items.add(
new File([window.atob(file.textContent)],
file.dataset.filename,
Expand Down
3 changes: 3 additions & 0 deletions share/roundup/templates/classic/html/page.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
columns_showall string:id,activity,title,creator,assignedto,status;
status_notresolved string:-1,1,2,3,4,5,6,7;
">
<div id="submit_overlay">
<div id="status_overlay_content">&#9203; Please wait...</div>
</div>
<header>
<div class="header-left">
<div id="header-logo">
Expand Down
21 changes: 19 additions & 2 deletions share/roundup/templates/classic/html/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ td.content {
width: 100%;
}

td.date, th.date {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you restore the trailing whitespace in the css and html files across all your changes. Looks like there are 3 of them. The whitespace fixups in the javascript in templating.py are fine. That's not something that has to be applied manually as part of the upgrade.

When I add updates like this to upgrading.txt, I sometimes include references to the commit. This lets
admins download the commit as a patch and apply it to their trackers. Having a clean patch
that only changes the one thing needed for the additional function is better.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My IDE removes all trailing spaces automatically upon save, I'll see what I can do to exempt your remo

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I can't apply your PR directly in github (our VCS of record is mercurial), so I can fix it before I commit. But having it fixed in the pull request would be best.

td.date, th.date {
white-space: nowrap;
}

Expand Down Expand Up @@ -412,7 +412,7 @@ table.classlist th {


/* style for class help display */
table.classhelp { /* the table-layout: fixed; */
table.classhelp { /* the table-layout: fixed; */
table-layout: fixed; /* compromises quality for speed */
overflow: hidden;
font-size: .9em;
Expand Down Expand Up @@ -514,5 +514,22 @@ textarea:focus, select:focus {
background-color: #afafaf;
}

.submit_overlay {
display: none;
position: fixed;
top: 0;
left: 0;
width: 100%;
height: 100%;
text-align: center;
place-content: center;
background-color: rgba(0,0,0,0.5);
z-index: 9999;
font-size: 200%;
font-weight: bold;
color: white;
text-shadow: 2px 2px 2px black;
}

/* vim: sts=2 sw=2 et
*/
5 changes: 4 additions & 1 deletion share/roundup/templates/devel/html/page.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
milestone_columns string:id,activity,title,creator,status;
milestone_columns_showall string:id,activity,title,creator,assignee,status;
milestone_status_notclosed string:-1,1,2,4;">
<div id="submit_overlay">
<div id="status_overlay_content">&#9203; Please wait...</div>
</div>
<div class="header">
<h1><a href="/">Roundup Demo Tracker</a></h1>
<div id="searchbox">
Expand Down Expand Up @@ -211,7 +214,7 @@ <h1><a href="/">Roundup Demo Tracker</a></h1>
<a tal:attributes="href string:user${request/user/id}" i18n:translate="">Your Details</a>
</li>
<li>
<a tal:attributes="href python:request.indexargs_url('', {'@action':'logout'})"
<a tal:attributes="href python:request.indexargs_url('', {'@action':'logout'})"
i18n:translate="">Logout</a>
</li>
<li class="" tal:condition="python:request.user.hasPermission('View', 'query')">
Expand Down
3 changes: 3 additions & 0 deletions share/roundup/templates/jinja2/html/layout/page.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
{%- block extracss %} {% endblock %}
</head>
<body>
<div id="submit_overlay">
<div id="status_overlay_content">&#9203; Please wait...</div>
</div>
{% block navbar %}
{% include 'layout/banner.html' %}
{% endblock %}
Expand Down
17 changes: 17 additions & 0 deletions share/roundup/templates/jinja2/static/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,20 @@ select:required {
border-width: 1.5px;
border-color: #6e747a;
}

.submit_overlay {
display: none;
position: fixed;
top: 0;
left: 0;
width: 100%;
height: 100%;
text-align: center;
place-content: center;
background-color: rgba(0,0,0,0.5);
z-index: 9999;
font-size: 200%;
font-weight: bold;
color: white;
text-shadow: 2px 2px 2px black;
}
2 changes: 1 addition & 1 deletion share/roundup/templates/minimal/html/page.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

</head>
<body class="body">

<span id="submit_overlay"></span>
<table class="body"
tal:define="
kw_edit python:request.user.hasPermission('Edit', 'keyword');
Expand Down
8 changes: 6 additions & 2 deletions share/roundup/templates/minimal/html/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ td.content {
width: 100%;
}

td.date, th.date {
td.date, th.date {
white-space: nowrap;
}

Expand Down Expand Up @@ -336,7 +336,7 @@ table.classlist th {


/* style for class help display */
table.classhelp { /* the table-layout: fixed; */
table.classhelp { /* the table-layout: fixed; */
table-layout: fixed; /* compromises quality for speed */
overflow: hidden;
font-size: .9em;
Expand Down Expand Up @@ -438,5 +438,9 @@ textarea:focus, select:focus {
background-color: #afafaf;
}

.submit_overlay {
display: none;
}

/* vim: sts=2 sw=2 et
*/
5 changes: 4 additions & 1 deletion share/roundup/templates/responsive/html/page.html
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
milestone_columns string:id,activity,title,creator,status;
milestone_columns_showall string:id,activity,title,creator,assignee,status;
milestone_status_notclosed string:-1,1,2,4;">
<div id="submit_overlay">
<div id="status_overlay_content">&#9203; Please wait...</div>
</div>
<div class="navbar navbar-fixed-top">
<div class='navbar-inner'>
<div class='container'>
Expand Down Expand Up @@ -224,7 +227,7 @@
<a tal:attributes="href string:user${request/user/id}" i18n:translate="">Your Details</a>
</li>
<li>
<a tal:attributes="href python:request.indexargs_url('', {'@action':'logout'})"
<a tal:attributes="href python:request.indexargs_url('', {'@action':'logout'})"
i18n:translate="">Logout</a>
</li>
<li class="" tal:condition="python:request.user.hasPermission('View', 'query')">
Expand Down
3 changes: 3 additions & 0 deletions website/issues/html/page.html
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
columns_showall string:id,activity,title,creator,assignee,status;
status_notresolved string:-1,1,2,4;
">
<div id="submit_overlay">
<div id="status_overlay_content">&#9203; Please wait...</div>
</div>
<div class="header">
<h1><a href="/">Roundup Tracker - Issues</a></h1>
<div id="searchbox">
Expand Down