Refining/simplifying jquery for hiding and showing fieldsets

Question

I'm starting to figure out this jquery stuff, but as you can see there's a long ways to go. How would I approach refining and simplifying this snippet. Its purpose is to show a fieldset[data-content] with inputs inside when you click a "show" link, and to hide said fieldset and remove all "required" attributes when you click the "Hide" link.

I'm using data-* for all functionality and .class for styling. Hence not using classes/ID's for these items.

JQuery

$(function() {
 $('[data-content]').hide(); // hide content initially
 $('[data-show]').click(function() {
 $('[data-content]').fadeIn('slow'); //fade in/show content
 $(this).addClass('active'); //add .active to "show" link
 $('[data-hide]').removeClass('active'); // remove .active from "hide" link
 $("[data-content] :input").prop('required',true); // add required if :visible
 return false;
 });
 $('[data-hide]').click(function() {
 $('[data-content]').fadeOut('fast'); //fade out/hide content
 $(this).addClass('active'); //add active class to "hide" link
 $('[data-show]').removeClass('active'); //remove active class from "show" link
 $("[data-content] :input").prop('required',null); // remove "required" if :hidden
 return false;
 });
});

HTML example

<a href="#" data-show class="active">Show</a>
<a href="#" data-hide>Hide</a>
<fieldset data-content>
 <input type="text" placeholder="Yada"><!-- only required if visible -->
</fieldset>
<fieldset>
 <input type="text" placeholder="yada2" required>
</fieldset>

Answer 1

1. Store selections in variables; don't call $(...) again and again for elements you've already selected
2. You start out by hiding the fieldset, but it'd be simpler to just hide it with CSS, so it starts out hidden by default. Conversely, if it's visible by default, then add the required attributes in the html, so it actually matches the behavior you want for a visible fieldset.
3. You're using data-* attributes, but not super effectively (see below).

The way you're using the data-* attributes is basically the same way you'd use a class or ID; as a key to help you find stuff. So you might as well use classes and IDs then, if you're not going to use the data-* attributes to store a value in addition to the key. But for cases like this, where you want a link to refer to an element, I suggest the following:

<a href="#someElement">Foo</a>
<div id="someElement">...</div>

The link is completely valid syntactically and semantically: It references an element.

Without JS intervening, it'll just scroll the page to element, but with JS, we can intercept the click and do the showing/hiding. Of course, there's no way to distinguish between hide and show links, but I think that should be a toggle-style action anyway.

But to keep to your setup, here's a way to do that (and now data-* attributes come in handy):

<a href="#" data-shows="#someElement">Show</a>
<a href="#" data-hides="#someElement">Hide</a>
<div id="someElement">...</div>

For the JS, you can then do something like this:

$(function() {
 var showLinks = $('a[data-shows]'),
 hideLinks = $('a[data-hides]');
 showLinks.on('click', function (event) {
 var target = $(this).addClass('active').data('shows');
 $(target).fadeIn().find(':input').prop('required', true);
 hideLinks.removeClass('active');
 return false;
 });
 hideLinks.on('click', function (event) {
 var target = $(this).addClass('active').data('hides');
 $(target).fadeOut().find(':input').prop('required', false);
 showLinks.removeClass('active');
 return false;
 });
});

There's a bit of repetition there, so you might do:

$(function() {
 var links = $('a[data-shows], a[data-hides]');
 links.on('click', function (event) {
 var link = $(this).addClass('active'),
 type = link.data('shows') ? 'shows' : 'hides',
 target = link.data(type),
 fadeFunc = type === 'shows' ? 'fadeIn' : "fadeOut";
 $(target)[fadeFunc]().find(':input').prop('required', type === 'shows');
 links.not(this).removeClass('active');
 return false;
 });
});

There are many ways of going about this

• darcher: This is phenomenal. It'll take me a while to break it down and fully understand everything. Had to update .prop('required', type !== 'shows'); as it was preventing the return false without the single quotes, but this definitely gives me a better idea of how to chain items using variables. Thanks!
• darcher: or ...type === 'shows'... rather
• Flambino: @darcher Ah, sorry - stupid mistake on my part. Glad you caught it