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.

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

add logic to toggle reducer keys based off user input #1610

Merged
merged 5 commits into from
Oct 10, 2024

Conversation

Tooyosi
Copy link
Contributor

@Tooyosi Tooyosi commented Oct 5, 2024

Solves for this ticket

Copy link
Contributor

@yuenmichelle1 yuenmichelle1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Toyosi!

Overall this looks good. But I'm unsure about having a "global" forms.js that will load in all our forms but whose logic will only work for the reducers form. I think we can have a <script> tag appended at the bottom of _form.html.erb that can deal with this logic, so that we can limit the scope.

If you are familiar with jQuery, we have access to jQuery in our rails app. (I'm a fan, but totally ok with vanilla JS)

I think for the reducer's_form.html.erb instead of using class='hidden' as initial default for subject_reducer_keys, we can do the following:

L18-25 of reducer's _form.html.erb we can declare the divs and form inputs. i.e.

<div id='user_reducer_keys'>
    <%= f.input :user_reducer_keys%>
 </div>
 <div id='subject_reducer_keys'>
    <%= f.input :subject_reducer_keys%>
 </div>

And then we can add a <script> tag on the bottom of reducer's _form.html.erb that can hide and show elements on load and on change. I.e.

<script type="text/javascript">
  function toggle_reducer_keys_inputs(topic) {
    if (topic.includes('subject')){
      $('#subject_reducer_keys').hide()
      $('#user_reducer_keys').show()
    } else {
      $('#subject_reducer_keys').show()
      $('#user_reducer_keys').hide()
    }
  }

  $(function() {
    var reducer_topic = $('#reducer_topic').val();
    toggle_reducer_keys_inputs(reducer_topic)
  });

  $('#reducer_topic').on('change', function(event){
    var reducer_topic = event.target.value;
    toggle_reducer_keys_inputs(reducer_topic)
  })
</script>

Something like that...it's been a hot sec since I've done any jQuery/javascript stuff, but I'm hoping that should work out 🤣

@@ -0,0 +1,36 @@
document.addEventListener("DOMContentLoaded", function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I question whether or not we need a form.js, because technically this javascript will load on all our forms, but the logic will only run for reducer form.

I think it might be better to have a <script> tag with your logic within the reducer's form partial (views/reducers/_form.html.erb).

Luckily we also have access to jquery, but I dont mind vanilla js as well :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, using script makes sense aswell, i was thinking views/forms incase we need other/extra form logic in future but this flies. I've modified the logic to use jquery now and it works just as you shared, vanilla js usuallly comes natural to me by default most times 😊

Copy link
Contributor

@yuenmichelle1 yuenmichelle1 Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh nice!! i literally just free coded that from what little i remember of js + jquery without testing whether or not that would work. 😆 .

@@ -131,4 +137,15 @@ def reducer_params(klass)
def record_not_valid(exception)
render json: { error: exception.message }, status: 422
end

def remove_unwanted_config_key(param_object)
Copy link
Contributor

@yuenmichelle1 yuenmichelle1 Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the name of this method a bit confusing. Maybe we can call it reset_config_reducer_keys(param_object)?

user_reducer_keys: 'user_reducer_keys_value',
subject_reducer_keys: 'subject_reducer_keys',
topic: 'reduce_by_subject'
},
Copy link
Contributor

@yuenmichelle1 yuenmichelle1 Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont forget to address the hound 🐶.

Hound comment for this one before was

Style/TrailingCommaInHashLiteral: Avoid comma after the last item of a hash.

@Tooyosi Tooyosi merged commit b07b52f into master Oct 10, 2024
2 checks passed
@Tooyosi Tooyosi deleted the add-reducer-keys-to-config-ui branch October 10, 2024 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants