PR#1309: frontend: disable build delete if js is not enabled - copr/copr

copr / copr

#1309 frontend: disable build delete if js is not enabled

Merged 4 years ago by praiskup. Opened 4 years ago by thrnciar.

copr/ thrnciar/copr disable-build-deletion-without-js into master

frontend: disable build delete if js is not enabled

Tomas Hrnciar • 4 years ago

b866853

frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_forms.html

file modified

+1 -1

		`@@ -202,7 +202,7 @@`
		`{% endmacro %}`

		`{% macro copr_delete_builds(copr, class="") %}`
		`- <form class="{{ class }} delete-builds" action="#" method="post">`
		`+ <form class="{{ class }} delete-builds show-me-javascript hidden" action="#" method="post">`
		`<button class="btn btn-danger" type="submit">`
		`<span class="pficon pficon-delete"></span> Delete all marked`
		`</button>`

frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_table.html

file modified

+3 -3

		`@@ -6,7 +6,7 @@`
		`{% macro builds_table(builds, print_possible_states=True) %}`
		`{% for build in builds %}`
		`{% if loop.first %}`
		`- <noscript><p>WARNING!! This page is using JavaScript to filter and sort the table.</p></noscript>`
		`+ <noscript><p>WARNING!! This page is using JavaScript to filter, sort and delete builds from the table.</p></noscript>`
		`<table class="datatable table table-striped table-bordered">`
		`<thead>`
		`<tr>`
		`@@ -18,7 +18,7 @@`
		`<th>Status</th>`

		`{% if g.user and g.user.can_edit(copr) %}`
		`- <th data-orderable="false"><a href="#" onclick="$('tr.build-row :checkbox').prop('checked', $('tr.build-row :checkbox').length != $('tr.build-row :checkbox:checked').length); return false;">Mark all</a></th>`
		`+ <th data-orderable="false" class="show-me-javascript hidden"><a href="#" onclick="$('tr.build-row :checkbox').prop('checked', $('tr.build-row :checkbox').length != $('tr.build-row :checkbox:checked').length); return false;">Mark all</a></th>`
		`{% endif %}`
		`</tr>`
		`</thead>`
		`@@ -53,7 +53,7 @@`
		`</td>`

		`{% if g.user and g.user.can_edit(copr) %}`
		`- <td>`
		`+ <td class="show-me-javascript hidden">`
		`<input type="checkbox" name="build_ids" value="{{ build.id }}"></input>`
		`</td>`
		`{% endif %}`

frontend/coprs_frontend/coprs/templates/layout.html

file modified

		`@@ -173,6 +173,7 @@`
		`<script>`
		`$(document).ready(function(){`
		`show_last_article( "{{ config.NEWS_FEED_URL }}" )`
		`+ $(".show-me-javascript").removeClass("hidden");`
		`});`
		`</script>`
		`{% endif %}`

thrnciar commented 4 years ago

Fixes: #1110

dturecek commented 4 years ago

praiskup commented on line 36 of frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_table.html 4 years ago

Can you please use something like td-build-{{ build.id }}? That will assure uniqueness.

praiskup commented 4 years ago

Otherwise works nice, thank you.

rebased onto d620262438b349592e7a096e745ab8e5cc1e0b74

4 years ago

praiskup commented on line 27 of frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_table.html 4 years ago

Hm, this snippet seems to be generated for each table row :-(. At least we should minimize this, and add only
the bytes which are needed (`"#delete-build-th" is redundant entirely), or make the function slightly more
clever (e.g. filter all elements by class?).

praiskup commented 4 years ago

E.g. iterate over array like document.querySelectorAll(".no-js-hidden");?

praiskup commented 4 years ago

Ah, there's https://api.jquery.com/class-selector/.

Metadata Update from @praiskup:
- Pull-request tagged with: needs-work

4 years ago

rebased onto c508b67d5935ff164ea0cd57b8eb343f8d0ae151

4 years ago

praiskup commented on line 24 of frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_table.html 4 years ago

Nice! We don't even have to define new class in css! :-).

To make it super obvious what is happening, could you please change the new class name to .show-me-javascript?

praiskup commented on line 24 of frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_table.html 4 years ago

Can you please move this to layout.html, so we can reuse this everywhere, there's already a proper place near show_last_article( "{{ config.NEWS_FEED_URL }}" ).

thrnciar commented 4 years ago

Thanks for the feedback, I'll do it.

praiskup commented on line 6 of frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_forms.html 4 years ago

Maybe we could use show-me-javascript hidden classes here, too?

rebased onto 6083c50f578e94f0186320e334bb6ba2e2ca44e7

4 years ago

rebased onto f8a61c570784f3c13a778e5b187144adf454f1e4

4 years ago

rebased onto b866853

4 years ago

thrnciar commented 4 years ago

@praiskup, PR was updated. PTAL