diff --git a/apps/dashboard/app/helpers/batch_connect/session_contexts_helper.rb b/apps/dashboard/app/helpers/batch_connect/session_contexts_helper.rb index 147a68f6e..0ff436849 100644 --- a/apps/dashboard/app/helpers/batch_connect/session_contexts_helper.rb +++ b/apps/dashboard/app/helpers/batch_connect/session_contexts_helper.rb @@ -7,6 +7,8 @@ def create_widget(form, attrib, format: nil, hide_excludable: true, hide_fixed: widget = attrib.widget field_options = attrib.field_options(fmt: format) all_options = attrib.all_options(fmt: format) + wrapper_present = field_options[:wrapper].is_a?(Hash) && field_options[:wrapper].present? + wrap_params = (wrapper_present ? field_options[:wrapper] : {}) if attrib.fixed? return render :partial => "batch_connect/session_contexts/fixed", :locals => { form: form, attrib: attrib, field_options: field_options, format: format } @@ -14,26 +16,26 @@ def create_widget(form, attrib, format: nil, hide_excludable: true, hide_fixed: rendered = case widget when 'select' - form.select(attrib.id, attrib.select_choices(hide_excludable: hide_excludable), field_options, attrib.html_options) + form.select(attrib.id, attrib.select_choices(hide_excludable: hide_excludable), field_options, attrib.all_options) when 'resolution_field' resolution_field(form, attrib.id, all_options) when 'check_box' - form.form_group attrib.id, help: field_options[:help] do + form.form_group(attrib.id, help: field_options[:help], **wrap_params) do form.check_box attrib.id, all_options, attrib.checked_value, attrib.unchecked_value end when 'radio', 'radio_button' - form.form_group attrib.id, help: field_options[:help] do + form.form_group(attrib.id, help: field_options[:help], **wrap_params) do opts = { label: label_tag(attrib.id, attrib.label), checked: (attrib.value.presence || attrib.field_options[:checked]) } - content_tag(:div, id: [form.object_name, attrib.id].join('_')) do + content_tag(:div, id: [form.object_name, attrib.id].join('_'), **field_options) do form.collection_radio_buttons(attrib.id, attrib.select_choices, :second, :first, **opts) end end when 'path_selector' - form.form_group(attrib.id) do - render(partial: 'path_selector', locals: { form: form, attrib: attrib, field_options: field_options }) + form.form_group(attrib.id, field_options[:wrapper]) do + render(partial: 'path_selector', locals: { form: form, attrib: attrib, field_options: field_options.except(:wrapper) }) end when 'file_attachments' render :partial => "batch_connect/session_contexts/file_attachments", :locals => { form: form, attrib: attrib, field_options: field_options } @@ -50,11 +52,12 @@ def create_widget(form, attrib, format: nil, hide_excludable: true, hide_fixed: end def resolution_field(form, id, opts = {}) - content_tag(:div, id: "#{id}_group", class: "mb3") do + wrapper_present = opts[:wrapper].is_a?(Hash) && opts[:wrapper].present? + content_tag(:div, id: "#{id}_group", class: "mb-3", **(wrapper_present ? opts[:wrapper] : {})) do concat form.label(id, opts[:label]) concat form.hidden_field(id, id: "#{id}_field") concat( - content_tag(:div, class: "row mb-3") do + content_tag(:div, id:[form.object_name, id].join('_'), class: "row mb-3", **opts) do concat ( content_tag(:div, class: "col-sm-6") do concat ( diff --git a/apps/dashboard/app/javascript/dynamic_forms.js b/apps/dashboard/app/javascript/dynamic_forms.js index bee2d4108..542e48b07 100644 --- a/apps/dashboard/app/javascript/dynamic_forms.js +++ b/apps/dashboard/app/javascript/dynamic_forms.js @@ -459,16 +459,17 @@ function updateVisibility(event, changeId) { }); if (changeElement === undefined || changeElement.length <= 0) return; - + + const defaultHidden = $(`#${changeId}`).attr('hide_by_default') || false const elementInfo = getWidgetInfo(changeId); // safe to access directly? const hide = hideLookup[id].get(changeId, val); - if ((hide === false) || (hide === undefined && !initializing)) { + if((hide === false) || (hide === undefined && !initializing && !defaultHidden)) { changeElement.show(); // Pass text into the aria stream const addMsg = `Revealed form item ${elementInfo}`; ariaStream(addMsg) - } else if (hide === true) { + } else if ((hide === true) || (hide === undefined && !initializing && defaultHidden)) { changeElement.hide(); const rmMsg = `Hid form item ${elementInfo}`; ariaStream(rmMsg); diff --git a/apps/dashboard/app/lib/smart_attributes/attribute.rb b/apps/dashboard/app/lib/smart_attributes/attribute.rb index 525b9715c..98a044248 100644 --- a/apps/dashboard/app/lib/smart_attributes/attribute.rb +++ b/apps/dashboard/app/lib/smart_attributes/attribute.rb @@ -93,6 +93,9 @@ def help_html(fmt: nil) OodAppkit.markdown.render(help(fmt: fmt)).html_safe end + def default_hide_wrapper + !opts[:hide_by_default] ? '' : {style: 'display: none'} + end # Whether this attribute is required # @return [Boolean] is required def required @@ -148,7 +151,8 @@ def field_options(fmt: nil) end.merge({ label: label(fmt: fmt), help: help_html(fmt: fmt), - required: required + required: required, + wrapper: default_hide_wrapper }) end diff --git a/apps/dashboard/test/system/batch_connect_test.rb b/apps/dashboard/test/system/batch_connect_test.rb index f0e6ece73..0a1bdd99f 100644 --- a/apps/dashboard/test/system/batch_connect_test.rb +++ b/apps/dashboard/test/system/batch_connect_test.rb @@ -881,7 +881,509 @@ def check_visibility(hidden_id, expect_hidden) data_hide_checkbox_test(form, 'checkbox_test', 'gpus', true) end + + def basic_default_hide_check_hidden(invert = false) + if invert + assert_selector("##{bc_ele_id('gpus')}") + assert_text('GPU type') + assert_text('refute this') + else + assert_selector("##{bc_ele_id('gpus')}", visible: :hidden) + refute_text('GPU type') + refute_text('refute this') + end + end + + def basic_hide_by_default_test(form, invert: false) + Dir.mktmpdir do |dir| + make_bc_app(dir, form) + visit new_batch_connect_session_context_url('sys/app') + + basic_default_hide_check_hidden(invert) + + select 'Account2', from: bc_ele_id('account') + basic_default_hide_check_hidden(!invert) + + select 'Account3', from: bc_ele_id('account') + basic_default_hide_check_hidden(invert) + + select 'Account4', from: bc_ele_id('account') + basic_default_hide_check_hidden(!invert) + + select 'Account1', from: bc_ele_id('account') + basic_default_hide_check_hidden(invert) + end + end + + test 'hide_by_default changes default behavior on check box' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: check_box + hide_by_default: true + label: 'GPU type' + help: 'refute this' + account: + widget: 'select' + options: + - ['Account1', 'account1'] + - ['Account2', 'account2', data-hide-gpus: false] + - ['Account3', 'account3'] + - ['Account4', 'account4', data-hide-gpus: false] + HEREDOC + basic_hide_by_default_test(form) + end + + test 'data-hide false overrides hide_by_default on check box' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: check_box + hide_by_default: true + label: 'GPU type' + help: 'refute this' + account: + widget: 'select' + options: + - ['Account1', 'account1', data-hide-gpus: false] + - ['Account2', 'account2'] + - ['Account3', 'account3', data-hide-gpus: false] + - ['Account4', 'account4'] + HEREDOC + basic_hide_by_default_test(form, invert: true) + end + + test 'hide_by_default changes default behavior on number field' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: number_field + hide_by_default: true + label: 'GPU type' + help: 'refute this' + account: + widget: 'select' + options: + - ['Account1', 'account1'] + - ['Account2', 'account2', data-hide-gpus: false] + - ['Account3', 'account3'] + - ['Account4', 'account4', data-hide-gpus: false] + HEREDOC + basic_hide_by_default_test(form) + end + + test 'data-hide false overrides hide_by_default on number field' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: check_box + hide_by_default: true + label: 'GPU type' + help: 'refute this' + account: + widget: 'select' + options: + - ['Account1', 'account1', data-hide-gpus: false] + - ['Account2', 'account2'] + - ['Account3', 'account3', data-hide-gpus: false] + - ['Account4', 'account4'] + HEREDOC + basic_hide_by_default_test(form, invert: true) + end + + test 'hide_by_default changes default behavior on radio button' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: 'radio' + hide_by_default: true + label: 'GPU type' + options: + - ["Type 1", "0"] + - ["Type 2", "1"] + account: + widget: 'select' + options: + - ['Account1', 'account1'] + - ['Account2', 'account2', data-hide-gpus: false] + HEREDOC + Dir.mktmpdir do |dir| + make_bc_app(dir, form) + visit new_batch_connect_session_context_url('sys/app') + + assert_selector("##{bc_ele_id('gpus')}", visible: :hidden) + assert_selector("##{bc_ele_id('gpus_0')}", visible: :hidden) + assert_selector("##{bc_ele_id('gpus_1')}", visible: :hidden) + + select 'Account2', from: bc_ele_id('account') + assert_selector("##{bc_ele_id('gpus')}") + assert_selector("##{bc_ele_id('gpus_0')}") + assert_selector("##{bc_ele_id('gpus_1')}") + + select 'Account1', from: bc_ele_id('account') + assert_selector("##{bc_ele_id('gpus')}", visible: :hidden) + assert_selector("##{bc_ele_id('gpus_0')}", visible: :hidden) + assert_selector("##{bc_ele_id('gpus_1')}", visible: :hidden) + end + end + + test 'data-hide false overrides hide_by_default on radio button' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: 'radio' + hide_by_default: true + label: 'GPU type' + options: + - ["Type 1", "0"] + - ["Type 2", "1"] + account: + widget: 'select' + options: + - ['Account1', 'account1', data-hide-gpus: false] + - ['Account2', 'account2'] + HEREDOC + Dir.mktmpdir do |dir| + make_bc_app(dir, form) + visit new_batch_connect_session_context_url('sys/app') + + assert_selector("##{bc_ele_id('gpus')}") + assert_selector("##{bc_ele_id('gpus_0')}") + assert_selector("##{bc_ele_id('gpus_1')}") + + select 'Account2', from: bc_ele_id('account') + assert_selector("##{bc_ele_id('gpus')}", visible: :hidden) + assert_selector("##{bc_ele_id('gpus_0')}", visible: :hidden) + assert_selector("##{bc_ele_id('gpus_1')}", visible: :hidden) + + select 'Account1', from: bc_ele_id('account') + assert_selector("##{bc_ele_id('gpus')}") + assert_selector("##{bc_ele_id('gpus_0')}") + assert_selector("##{bc_ele_id('gpus_1')}") + end + end + + def assert_resolution_field_visibility(field, visible:) + opts = (visible ? {} : { visible: :hidden }) + + assert_selector("##{field}_group", **opts) + assert_selector("##{bc_ele_id(field)}", **opts) + assert_selector("##{field}_x_field", **opts) + assert_selector("##{field}_y_field", **opts) + end + + test 'hide_by_default changes default behavior on resolution field' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: resolution_field + hide_by_default: true + label: 'GPU type' + help: 'refute this' + account: + widget: 'select' + options: + - ['Account1', 'account1'] + - ['Account2', 'account2', data-hide-gpus: false] + - ['Account3', 'account3'] + - ['Account4', 'account4', data-hide-gpus: false] + HEREDOC + # Test help and label first, then specific resolution_field structure + basic_hide_by_default_test(form) + + Dir.mktmpdir do |dir| + make_bc_app(dir, form) + visit new_batch_connect_session_context_url('sys/app') + assert_resolution_field_visibility('gpus', visible: false) + + select 'Account2', from: bc_ele_id('account') + assert_resolution_field_visibility('gpus', visible: true) + + select 'Account3', from: bc_ele_id('account') + assert_resolution_field_visibility('gpus', visible: false) + + select 'Account4', from: bc_ele_id('account') + assert_resolution_field_visibility('gpus', visible: true) + + select 'Account1', from: bc_ele_id('account') + assert_resolution_field_visibility('gpus', visible: false) + end + end + + test 'data-hide false overrides hide_by_default on resolution field' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: resolution_field + hide_by_default: true + label: 'GPU type' + help: 'refute this' + account: + widget: 'select' + options: + - ['Account1', 'account1', data-hide-gpus: false] + - ['Account2', 'account2'] + - ['Account3', 'account3', data-hide-gpus: false] + - ['Account4', 'account4'] + HEREDOC + # Test help and label first, then specific resolution_field structure + basic_hide_by_default_test(form, invert: true) + Dir.mktmpdir do |dir| + make_bc_app(dir, form) + visit new_batch_connect_session_context_url('sys/app') + assert_resolution_field_visibility('gpus', visible: true) + + select 'Account2', from: bc_ele_id('account') + assert_resolution_field_visibility('gpus', visible: false) + select 'Account3', from: bc_ele_id('account') + assert_resolution_field_visibility('gpus', visible: true) + + select 'Account4', from: bc_ele_id('account') + assert_resolution_field_visibility('gpus', visible: false) + + select 'Account1', from: bc_ele_id('account') + assert_resolution_field_visibility('gpus', visible: true) + end + end + + test 'hide_by_default changes default behavior on select' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: select + hide_by_default: true + label: 'GPU type' + help: 'refute this' + options: + - ['Type 1'] + - ['Type 2'] + account: + widget: 'select' + options: + - ['Account1', 'account1'] + - ['Account2', 'account2', data-hide-gpus: false] + - ['Account3', 'account3'] + - ['Account4', 'account4', data-hide-gpus: false] + HEREDOC + basic_hide_by_default_test(form) + end + + test 'data-hide false overrides hide_by_default on select' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: select + hide_by_default: true + label: 'GPU type' + help: 'refute this' + options: + - ['Type 1'] + - ['Type 2'] + account: + widget: 'select' + options: + - ['Account1', 'account1', data-hide-gpus: false] + - ['Account2', 'account2'] + - ['Account3', 'account3', data-hide-gpus: false] + - ['Account4', 'account4'] + HEREDOC + basic_hide_by_default_test(form, invert: true) + end + + test 'hide_by_default changes default behavior on text field' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: text_field + hide_by_default: true + label: 'GPU type' + help: 'refute this' + account: + widget: 'select' + options: + - ['Account1', 'account1'] + - ['Account2', 'account2', data-hide-gpus: false] + - ['Account3', 'account3'] + - ['Account4', 'account4', data-hide-gpus: false] + HEREDOC + basic_hide_by_default_test(form) + end + + test 'data-hide false overrides hide_by_default on text field' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: text_field + hide_by_default: true + label: 'GPU type' + help: 'refute this' + account: + widget: 'select' + options: + - ['Account1', 'account1', data-hide-gpus: false] + - ['Account2', 'account2'] + - ['Account3', 'account3', data-hide-gpus: false] + - ['Account4', 'account4'] + HEREDOC + basic_hide_by_default_test(form, invert: true) + end + + test 'hide_by_default changes default behavior on path selector' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: path_selector + hide_by_default: true + label: 'GPU type' + help: 'refute this' + account: + widget: 'select' + options: + - ['Account1', 'account1'] + - ['Account2', 'account2', data-hide-gpus: false] + - ['Account3', 'account3'] + - ['Account4', 'account4', data-hide-gpus: false] + HEREDOC + Dir.mktmpdir do |dir| + make_bc_app(dir, form) + visit new_batch_connect_session_context_url('sys/app') + refute_text('Select Path') + + select 'Account2', from: bc_ele_id('account') + assert_text('Select Path') + + select 'Account3', from: bc_ele_id('account') + refute_text('Select Path') + + select 'Account4', from: bc_ele_id('account') + assert_text('Select Path') + + select 'Account1', from: bc_ele_id('account') + refute_text('Select Path') + end + end + + test 'data-hide false overrides hide_by_default on path selector' do + form = <<~HEREDOC + --- + cluster: + - owens + form: + - gpus + - account + attributes: + gpus: + widget: path_selector + hide_by_default: true + label: 'GPU type' + help: 'refute this' + account: + widget: 'select' + options: + - ['Account1', 'account1', data-hide-gpus: false] + - ['Account2', 'account2'] + - ['Account3', 'account3', data-hide-gpus: false] + - ['Account4', 'account4'] + HEREDOC + basic_hide_by_default_test(form, invert: true) + Dir.mktmpdir do |dir| + make_bc_app(dir, form) + visit new_batch_connect_session_context_url('sys/app') + assert_text('Select Path') + + select 'Account2', from: bc_ele_id('account') + refute_text('Select Path') + + select 'Account3', from: bc_ele_id('account') + assert_text('Select Path') + + select 'Account4', from: bc_ele_id('account') + refute_text('Select Path') + + select 'Account1', from: bc_ele_id('account') + assert_text('Select Path') + end + end + test 'options with hyphens set min & max' do visit new_batch_connect_session_context_url('sys/bc_jupyter')