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

Limit shuffling of fieldset children #304

Merged

Conversation

jessouyangbraven
Copy link
Contributor

Task: https://app.asana.com/0/1170776727341290/1175471955522877, https://app.asana.com/0/1170776727341290/1175281876558207

Test

  • Save and Publish content to local canvasweb
  • I had to use a new incognito window every time to test JS changes so the browser wouldn't cache it

Inputs we don't shuffle:

Screen Shot 2020-05-15 at 3 22 12 PM

Inputs we do want to shuffle:

  • Plain old checklist
  • Plain old radio

Screen Shot 2020-05-15 at 3 12 35 PM

Details

  • I didn't use the input[type=radio], input[type=checkbox] selector because the inputs in the fieldset are actually wrapped in <div>s to accommodate things like inline feedback, etc., so we are dependent on these specific classnames being in the HTML

// Mix up radio and checklists unless marked dont-mix
jQuery('fieldset').not('.dont-mix').each(function() {
var fieldset = jQuery(this);
var validChildren = fieldset.children('.module-checkbox-div, .module-radio-div');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've never written jQuery before. I think this is the correct selector syntax?

Copy link
Contributor

@rshipp rshipp left a comment

Choose a reason for hiding this comment

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

lgtm!

@jessouyangbraven jessouyangbraven merged commit 68d4695 into beyond-z:staging May 18, 2020
@jessouyangbraven jessouyangbraven deleted the editor-shufflejs branch May 18, 2020 16:18
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