Skip to content
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.

Sign up for GitHub

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

4013 data exclusive option for #4025

Merged
merged 7 commits into from
Dec 20, 2024
Merged

4013 data exclusive option for #4025

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7f1bdb4
Adds basic test for data-option-for-*
HazelGrant Dec 18, 2024
4205ae7
Adds failing test for data-option-exclusive-for-*
HazelGrant Dec 18, 2024
5875584
Adds code to make the broken test pass
HazelGrant Dec 19, 2024
3079c85
Refactoring
HazelGrant Dec 19, 2024
a956932
Small refactor to match code style & additional assertions to test mi…
HazelGrant Dec 19, 2024
eeba467
Moves instiation lines for optionFor handlers to optionForHandler fun…
HazelGrant Dec 20, 2024
bc0f1ad
Matches code style to existing tests
HazelGrant Dec 20, 2024
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
93 changes: 70 additions & 23 deletions apps/dashboard/app/javascript/dynamic_forms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const formTokens = [];
// elements. I.e., {'cluster': [ 'node_type' ] } means that changes to cluster
// trigger changes to node_type
const optionForHandlerCache = {};
const exclusiveOptionForHandlerCache = {};


// simples array of string ids for elements that have a handler
Expand Down Expand Up @@ -109,7 +110,6 @@ function snakeCaseWords(str) {
function memorizeElements(elements) {
elements.each((_i, ele) => {
formTokens.push(mountainCaseWords(shortId(ele['id'])));
optionForHandlerCache[ele['id']] = [];
});
};

Expand Down Expand Up @@ -137,6 +137,9 @@ function makeChangeHandlers(prefix){
if(key.startsWith('optionFor')) {
let token = key.replace(/^optionFor/,'');
addOptionForHandler(idFromToken(token), element['id']);
} else if (key.startsWith('exclusiveOptionFor')) {
let token = key.replace(/^exclusiveOptionFor/, '');
addExclusiveOptionForHandler(idFromToken(token), element['id']);
} else if(key.startsWith('max') || key.startsWith('min')) {
addMinMaxForHandler(element['id'], opt.value, key, data[key]);
} else if(key.startsWith('set')) {
Expand Down Expand Up @@ -535,10 +538,19 @@ function clamp(currentValue, previous, next) {
}
}

function addOptionForHandler(causeId, targetId) {
function sharedOptionForHandler(causeId, targetId, optionForType) {
const changeId = String(causeId || '');

if(changeId.length == 0 || optionForHandlerCache[causeId].includes(targetId)) {
let handlerCache = null;

if (optionForType == 'optionFor') {
if (optionForHandlerCache[causeId] == undefined) optionForHandlerCache[causeId] = [];
handlerCache = optionForHandlerCache;
} else if (optionForType == 'exclusiveOptionFor') {
if (exclusiveOptionForHandlerCache[causeId] == undefined) exclusiveOptionForHandlerCache[causeId] = [];
handlerCache = exclusiveOptionForHandlerCache;
}

if(changeId.length == 0 || handlerCache[causeId].includes(targetId)) {
// nothing to do. invalid causeId or we already have a handler between the 2
return;
}
Expand All @@ -547,15 +559,31 @@ function addOptionForHandler(causeId, targetId) {

if(targetId && causeElement) {
// cache the fact that there's a new handler here
optionForHandlerCache[causeId].push(targetId);
handlerCache[causeId].push(targetId);

causeElement.on('change', (event) => {
toggleOptionsFor(event, targetId);
if (optionForType == 'exclusiveOptionFor') {
toggleExclusiveOptionsFor(event, targetId);
} else if (optionForType == 'optionFor') {
toggleOptionsFor(event, targetId);
}
});

// fake an event to initialize
toggleOptionsFor({ target: document.querySelector(`#${causeId}`) }, targetId);
if (optionForType == 'exclusiveOptionFor') {
toggleExclusiveOptionsFor({ target: document.querySelector(`#${causeId}`) }, targetId);
} else if (optionForType == 'optionFor') {
toggleOptionsFor({ target: document.querySelector(`#${causeId}`) }, targetId);
}
}
}

function addOptionForHandler(causeId, targetId) {
sharedOptionForHandler(causeId, targetId, 'optionFor');
};

function addExclusiveOptionForHandler(causeId, targetId) {
sharedOptionForHandler(causeId, targetId, 'exclusiveOptionFor');
};

function parseCheckedWhen(key) {
Expand Down Expand Up @@ -687,19 +715,19 @@ function idFromToken(str) {
}
}


/**
* Extract the option for out of an option for directive.
*
* @example
* optionForClusterOakley -> Cluster
* exclusiveOptionForClusterOakley -> Cluster
*
* @param {*} str
* @returns - the option for string
*/
function optionForFromToken(str) {
function sharedOptionForFromToken(str, optionForType) {
return formTokens.map((token) => {
let match = str.match(`^optionFor${token}`);
let match = str.match(`^${optionForType}${token}`);

if (match && match.length >= 1) {
return token;
Expand All @@ -709,14 +737,15 @@ function optionForFromToken(str) {
})[0];
}

/**
* Hide or show options of an element based on which cluster is
* currently selected and the data-option-for-CLUSTER attributes
* for each option
*
* @param {string} element_name The name of the element with options to toggle
*/
function toggleOptionsFor(_event, elementId) {
function optionForFromToken(str) {
return sharedOptionForFromToken(str, 'optionFor');
}

function exclusiveOptionForFromToken(str) {
return sharedOptionForFromToken(str, 'exclusiveOptionFor');
}

function sharedToggleOptionsFor(_event, elementId, contextStr) {
const options = [...document.querySelectorAll(`#${elementId} option`)];
let hideSelectedValue = undefined;

Expand All @@ -727,11 +756,17 @@ function optionForFromToken(str) {
// something else entirely. We're going to hide this option if _any_ of the
// option-for- directives apply.
for (let key of Object.keys(option.dataset)) {
let optionFor = optionForFromToken(key);
let optionForId = idFromToken(key.replace(/^optionFor/,''));
let optionFor = '';

if (contextStr == 'optionFor') {
optionFor = optionForFromToken(key);
} else if (contextStr == 'exclusiveOptionFor') {
optionFor = exclusiveOptionForFromToken(key);
}
let optionForId = idFromToken(key.replace(new RegExp(`^${contextStr}`),''));

// it's some other directive type, so just keep going and/or not real
if(!key.startsWith('optionFor') || optionForId === undefined) {
if(!key.startsWith(contextStr) || optionForId === undefined) {
continue;
}

Expand All @@ -742,7 +777,12 @@ function optionForFromToken(str) {
optionForValue = `-${optionForValue}`;
}

hide = option.dataset[`optionFor${optionFor}${optionForValue}`] === 'false';
if (contextStr == 'optionFor') {
hide = option.dataset[`optionFor${optionFor}${optionForValue}`] === 'false';
} else if (contextStr == 'exclusiveOptionFor') {
hide = !(option.dataset[`exclusiveOptionFor${optionFor}${optionForValue}`] === 'true')
}

if (hide) {
break;
}
Expand Down Expand Up @@ -796,8 +836,15 @@ function optionForFromToken(str) {

// now that we're done, propogate this change to data-set or data-hide handlers
document.getElementById(elementId).dispatchEvent((new Event('change', { bubbles: true })));
};
}

function toggleOptionsFor(_event, elementId) {
sharedToggleOptionsFor(_event, elementId, 'optionFor');
}

function toggleExclusiveOptionsFor(_event, elementId) {
sharedToggleOptionsFor(_event, elementId, 'exclusiveOptionFor');
};

export {
makeChangeHandlers
Expand Down
106 changes: 106 additions & 0 deletions apps/dashboard/test/system/batch_connect_widgets_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,4 +431,110 @@ def make_bc_app(dir, form)
end
end
end

test 'data-options-for-' do
Dir.mktmpdir do |dir|
"#{dir}/app".tap { |d| Dir.mkdir(d) }
SysRouter.stubs(:base_path).returns(Pathname.new(dir))
stub_scontrol
stub_sacctmgr
stub_git("#{dir}/app")

form = <<~HEREDOC
---
form:
- cluster
- node_type
attributes:
cluster:
widget: "select"
options:
- owens
- pitzer
node_type:
widget: "select"
options:
- standard
- ['gpu', 'gpu', data-option-for-cluster-pitzer: false]
HEREDOC

Pathname.new("#{dir}/app/").join('form.yml').write(form)
base_id = 'batch_connect_session_context_path'

visit new_batch_connect_session_context_url('sys/app')

# owens is selected, standard and gpu are both visible
select('owens', from: 'batch_connect_session_context_cluster')
options = find_all("#batch_connect_session_context_node_type option")

assert_equal "standard", options[0]["innerHTML"]
assert_equal '', find_option_style('node_type', 'gpu')

# select gpu, to test that it's deselected properly when pitzer is selected
select('gpu', from: 'batch_connect_session_context_node_type')

# pitzer is selected, gpu is not visible
select('pitzer', from: 'batch_connect_session_context_cluster')
options = find_all("#batch_connect_session_context_node_type option")

assert_equal "standard", options[0]["innerHTML"]
assert_equal 'display: none;', find_option_style('node_type', 'gpu')

# value of node_type has gone back to standard
assert_equal 'standard', find('#batch_connect_session_context_node_type').value
end
end

test 'data-option-exlusive-for-' do
Dir.mktmpdir do |dir|
"#{dir}/app".tap { |d| Dir.mkdir(d) }
SysRouter.stubs(:base_path).returns(Pathname.new(dir))
stub_scontrol
stub_sacctmgr
stub_git("#{dir}/app")

form = <<~HEREDOC
---
form:
- cluster
- node_type
attributes:
cluster:
widget: "select"
options:
- owens
- pitzer
node_type:
widget: "select"
options:
- standard
- ['gpu', 'gpu', data-exclusive-option-for-cluster-owens: true]
HEREDOC

Pathname.new("#{dir}/app/").join('form.yml').write(form)
base_id = 'batch_connect_session_context_path'

visit new_batch_connect_session_context_url('sys/app')

# owens is selected, standard and gpu are both visible
select('owens', from: 'batch_connect_session_context_cluster')
options = find_all("#batch_connect_session_context_node_type option")

assert_equal "standard", options[0]["innerHTML"]
assert_equal '', find_option_style('node_type', 'gpu')

# select gpu, to test that it's deselected properly when pitzer is selected
select('gpu', from: 'batch_connect_session_context_node_type')

# pitzer is selected, gpu is not visible
select('pitzer', from: 'batch_connect_session_context_cluster')
options = find_all("#batch_connect_session_context_node_type option")

assert_equal "standard", options[0]["innerHTML"]
assert_equal 'display: none;', find_option_style('node_type', 'gpu')

# value of node_type has gone back to standard
assert_equal 'standard', find('#batch_connect_session_context_node_type').value
end
end
end
Loading