JM - (SPARCDashboard bootstrap 4) Epic Queue & Protocol Merge Pages #1966
Conversation
jwiel86
changed the title
| @@ -20,18 +20,24 @@ | |||
| .panel.panel-default#epic-queue-panel | |||
There was a problem hiding this comment.
Can you remove the panel and the heading/body since they no longer exist with Bootstrap 4 and replace it with a column div with mx-auto to center it? A div with col-12 col-sm-10 mx-auto is what the notifications page is using.
| @@ -18,22 +18,22 @@ | |||
| -# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR~ | |||
| -# TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.~ | |||
|
|
|||
| .bootstrap-table-dropdown-overflow | |||
| .card | |||
| #epic-queues-custom-toolbar | |||
| %table.epic-queue-records-table{ data: { toggle: 'table', search: 'true', 'show-columns' => 'true', 'show-refresh' => 'true', 'show-toggle' => 'true', 'show-export' => 'true', pagination: 'true', side_pagination: 'server', url: dashboard_epic_queue_records_path, striped: 'true', toolbar: '#epic-queues-custom-toolbar' } } | |||
| %thead.primary-header | |||
| %tr | |||
| %th{data: { field: "protocol", align: "left", sortable: 'true', class: 'text-primary' } } | |||
There was a problem hiding this comment.
Can you remove the text-primary class from this th? It makes it seem like there should be a tooltip or some sort of link but there isn't
| @@ -18,22 +18,22 @@ | |||
| -# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR~ | |||
| -# TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.~ | |||
|
|
|||
| .bootstrap-table-dropdown-overflow | |||
| .card | |||
| #epic-queues-custom-toolbar | |||
| %table.epic-queue-records-table{ data: { toggle: 'table', search: 'true', 'show-columns' => 'true', 'show-refresh' => 'true', 'show-toggle' => 'true', 'show-export' => 'true', pagination: 'true', side_pagination: 'server', url: dashboard_epic_queue_records_path, striped: 'true', toolbar: '#epic-queues-custom-toolbar' } } | |||
| %thead.primary-header | |||
There was a problem hiding this comment.
Can you replace the primary-header class with bg-light? The <theme-color>-header classes no longer exist and we're now using bg-light to style theads
| @@ -18,24 +18,24 @@ | |||
| -# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR~ | |||
| -# TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.~ | |||
|
|
|||
| .bootstrap-table-dropdown-overflow | |||
| .card | |||
| #epic-queues-custom-toolbar | |||
| %table.epic-queue-table{ data: { toggle: 'table', search: 'true', 'show-columns' => 'true', 'show-refresh' => 'true', 'show-toggle' => 'true', pagination: 'true', side_pagination: 'server', url: dashboard_epic_queues_path, striped: 'true', toolbar: '#epic-queues-custom-toolbar' } } | |||
| %thead.primary-header | |||
There was a problem hiding this comment.
See previous comments about the thead and text-primary
| %th{data: { field: "status", align: "left", sortable: 'true' } } | ||
| = t(:epic_queues)[:status] | ||
| = t(:dashboard)[:epic_queues][:status] | ||
| %th{data: { field: 'send', align: 'center' } } |
There was a problem hiding this comment.
Can you combine the send and delete columns into a single actions column (you can use t(:actions)[:actions] for YAML) to be consistent with other tables in the application
| @@ -3,7 +3,7 @@ json.rows do | |||
| json.(@epic_queue_records.limit(params[:limit]).offset(params[:offset])) do |eqr| | |||
| json.protocol_id eqr.protocol_id | |||
| json.protocol format_protocol(eqr.protocol) | |||
| json.notes notes_button(eqr) | |||
| json.notes notes_button(eqr, protocol_id: eqr.protocol.id) | |||
| json.pis format_pis(eqr.protocol) | |||
| json.date format_epic_queue_created_at(eqr) | |||
| json.status eqr.status.capitalize | |||
There was a problem hiding this comment.
See comment about combining send and delete into actions, can you make that change here as well. You can add a helper that joins the send and delete buttons (see authorized_user_actions, document_actions, etc. for examples)
| data: { protocol_id: epic_queue.protocol.id, permission: 'true', | ||
| eq_id: epic_queue.id }, | ||
| class: 'btn btn-success push-to-epic') | ||
| link_to icon('fas', 'hand-point-right'), push_to_epic_protocol_path(epic_queue.protocol.id, eq_id: epic_queue.id, from_portal: true, format: :js), remote: true, method: :get, class: 'btn btn-success push-to-epic', data: { permission: 'true' } |
There was a problem hiding this comment.
Can you remove the from_portal: true param because it's not used in the controller and the format: :js param because remote: true is how you tell a link_to to use the JS format
https://www.pivotaltracker.com/story/show/166643642