Launcher save fixed attributes #3784
Conversation
|
|
||
| if (labelString.match(/Environment( |\s)Variable/)) { | ||
| var label_field = event.target.parentElement.children[2].children[0]; | ||
| label_field.innerHTML = `Environment Variable: ${aev_name}`; | ||
| } | ||
|
|
||
| // Update the checkbox so that environment variables can be fixed when created | ||
| let fixedBoxGroup = event.target.parentElement.children[3].children[0].children[0]; |
There was a problem hiding this comment.
This looks like it's really flaky and prone to errors. Is there some ID or data- attribute we can apply here so we don't have this?
querySelector (or querySelectorAll) is a method on the document object, bit it's also a method on all elements. So you could do event.target.querySelector to find what we're looking for.
ashton22305
force-pushed
the
save-fixed-attributes
branch
from
8171a5a to
43289e6
Compare
| def self.default_script_value(initial_opts, script_opts) | ||
| return nil if !initial_opts[:value] || script_opts.empty? | ||
|
|
||
| # Replace directory if the script is present in correct directory, otherwise delete value | ||
| valid_opt = script_opts.select { |opt| opt.first == File.basename(initial_opts[:value]) }.first | ||
| if valid_opt | ||
| initial_opts[:value] = valid_opt.last | ||
| elsif script_opts&.none? { |opt| opt.include?(initial_opts[:value]) } | ||
| initial_opts.delete(:value) | ||
| end | ||
|
|
||
| (initial_opts[:value] || script_opts.first.last).to_s | ||
| end |
There was a problem hiding this comment.
I'm struggling to understand this. I think because it does 2 things - potentially modifies initial_opts and has a return value.
Can we restructure this so it just has the return value with no modification to the things passed into it?
When I was investigating the tests, I came up with this - I also think a simple name change (and type change of the first argument) helped me figure out what it's attempting to do.
# called through default_value(opts[:default], options)
def self.default_value(default, scripts)
if scripts.empty?
nil
elsif default.nil? || default.to_s.empty?
scripts.first[1]
elsif scripts.map { |s| s[1] }.include?(default)
default
else
scripts.first[1]
end
end
There was a problem hiding this comment.
Can you submit the changes to this file in another pull request? I can't just glance at this to see what it is, so I'll need more time to look it over.
Fixes #3743
Prevents
bc_num_slotsfrom overwritingoptsPrevents
auto_coresfrom being overwritten entirely in the launcherUpdates the
data-fixed-togglerattribute forauto-environment-variablewhen the environment variable changes