Repository navigation
Hacky submit_once status overlay #135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3442,15 +3442,32 @@ def base_javascript(self): | |
| return """ | ||
| <script nonce="%s" type="text/javascript"> | ||
| submitted = false; | ||
| function submit_once() { | ||
| submitted_restore = ""; | ||
|
|
||
| function submit_reset() { | ||
| $("#submit_overlay").hide(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. jquery has some specific uses for some interactions and was added back |
||
| if (typeof(submitted)!=typeof(true)) { | ||
| submitted.value = submitted.textContent = submitted_restore; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. value and text content might not be the same specially if the text content is multiple words.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
| } | ||
|
|
||
|
|
@@ -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 => | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,7 +174,7 @@ td.content { | |
| width: 100%; | ||
| } | ||
|
|
||
| td.date, th.date { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
| } | ||
|
|
||
|
|
@@ -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; | ||
|
|
@@ -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 | ||
| */ | ||
There was a problem hiding this comment.
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
submittedvariable two different meanings: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
submittedas a simple boolean value.Since you added the variable:
submitted_restore, make it have one of two values:null(which can be tested withsubmitted_restore !== null(which should test type as well as value)Doing it this way, you restore the
valueif it is not the same as the text string. This happens in the SubmitSilentChange example.