4
\$\begingroup\$

I have a page with a few options that the owner of a site needs to be able to set. He's not a coder and I'm trying to make it as user-friendly as possible.

It works, but my implementation feels sloppy and I was wondering if there's a better way to do it.

This first part from my main javascript file will be minified and the site owner won't even need to look at it. The unminified source might be passed on to another developer in the future, though:

/* If this Squarespace page title isn't in the onlyShowOn list of pages,
 * stop now. */
var pageTitle = $('meta[property="og:title"]').attr('content');
if (pageTitle !== undefined && onlyShowOn.indexOf(pageTitle) == -1 ) {
    return true;
}

if (pageOptions.hasOwnProperty(pageTitle) && pageOptions[pageTitle] !== undefined) {
    var pageOption = pageOptions[pageTitle];
    if (pageOption.hasOwnProperty('skipHeadings') && pageOption.skipHeadings !== undefined) {
        skipHeadings = pageOption.skipHeadings;
    }

    if (pageOption.hasOwnProperty('headingSubstitution') && pageOption.headingSubstitution !== undefined) {
        headingSubstitution = pageOption.headingSubstitution;
    }
}

I don't like all the hasOwnProperty and checking for undefined, but I want to cover the cases where he doesn't set the options for all his pages or doesn't set all of the options.

Here's the options file that he will modify before injecting all of the code into the footer of his Squarespace site:

/* skipHeadings:
    This variable is used to exclude headings from the ToC. For example, a
    value of 2 will exclude the first 2 headings from the ToC.  */
var skipHeadings = 1;

/* headingSubstitution:
    This variable is used to provide substitutions which will show in the
    ToC. Any heading that matches exactly the quoted text before the colon
    will appear in the table of contents using the quoted text after the
    colon.
     - The values have to be quoted.
     - The heading and the substitution have to be separated by a colon.
     - The substitutions have to be separated by commas.  */
var headingSubstitution = {
    'A really long heading to test box size issues.': 'Long Heading ...',
    'Playing with Tocify': 'Playing ...' // You can add comments here, too.
};

/* onlyShowOn:
    This variable contains the list of pages on which the ToC will show.
     - The values have to be quoted, separated by commas, and match the name
       of your page in Squarespace exactly.
     - You can use either single- or double-quotes.
     - The entries can be on separate lines as long as they're separated by
       commas.  */
var onlyShowOn = [
    'Some Other Page',  // You can add comments after the page names like this.
    'Tocify Test Page', // The name of the page I'm using on my development server.
    'Tocify Page', // The name of the page I'm using on my Squarespace test site.
];

/* pageOptions:
    This variable contains per-page options for skipHeadings and
    headingSubstitution. The values above are used as defaults if a value is
    set here, it completely overrides the default value. As with the onlyShowOn
    option, the page name must match the name of your page in Squarespace
    exactly. The rules for the value definitions are the same as for the
    default options. */
var pageOptions = {
    'Some Other Page': {
        'skipHeadings': 2,
        'headingSubstitution': {
            'A really long heading to test box size issues.': 'Long Heading ...',
        }
    },
    'Tocify Test Page': {
    },
    'Tocify Page': {
    }
};
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Firstly, good job in commenting. The thing I usually look for in comments is why this thing is needed, which you really did in detail. However, your comments might need a bit of trimming. Explaining is good, making them too long though makes them visually distracting.

The onlyShowOn and pageOptions structures are redundant. You can just use pageOptions instead to check for existence.

var pageTitle = $('meta[property="og:title"]').attr('content');

I believe this can be replaced by the following, removing the dependency to jQuery.

var pageTitle = document.querySelector('meta[property="og:title"]').getAttribute('content');

In the end, your code is about getting skipHeadings and headingSubstitution based on the value of the title and falling back to default values if some parameters don't meet. So let's start with that.

var defaults = {
  skipHeadings: 1,
  headingSubstitution: {
    'A really long heading to test box size issues.': 'Long Heading ...',
    'Playing with Tocify': 'Playing ...'
  }
};

var substitutes = {
  'Some Other Page': {
    skipHeadings: 2,
    headingSubstitution: {
      'A really long heading to test box size issues.': 'Long Heading ...',
    }
  },
  'Tocify Test Page': {},
  'Tocify Page': {}
};


var pageTitle = document.querySelector('meta[property="og:title"]').getAttribute('content');

var hasSubstitute = pageOptions.hasOwnProperty(pageTitle) && substitutes[pageTitle];

// We check if there's a substitute. If not, use the default. Also, if our
// substitute has missing properties, Object.assign to fills them in with
// defaults.
var substitute = hasSubstitute ? Object.assign({}, defaults, substitutes[pageTitle]): defaults;

// In the end, substitute will always have a skipHeadings and
// headingSubstitution property, allowing us to write without checks.
var skipHeadings = substitute.skipHeadings;
var headingSubstitution = substitute.headingSubstitution;
\$\endgroup\$
4
  • \$\begingroup\$ Excellent! I didn't even know about Object.assign(). I'll take your advice; make the comments more concise, use the per-page options in place of onlyShowOn, and use Object.assign(). The project has a jQuery dependency anyway, but I think I'll go with the document.querySelector() as it's better practice for the future. \$\endgroup\$ Commented Mar 13, 2016 at 6:40
  • \$\begingroup\$ Unfortunately, no support for Object.assign() in IE11 is a deal-breaker for this particular project. I'll have to iterate over the object. CanIUse links to this page instead of showing its usually chart. \$\endgroup\$ Commented Mar 13, 2016 at 6:57
  • 1
    \$\begingroup\$ @Vince There's a shim for ES6 methods that includes Object.assign. It even comes with a standalone version if you don't want to have everything else. Also, seeing you're using jQuery, $.extend does the same thing. \$\endgroup\$ Commented Mar 13, 2016 at 16:05
  • \$\begingroup\$ @Vince And if jQuery is a hard dependency, then by all means use $() for getting elements. It's better for cross-browser situations. \$\endgroup\$ Commented Mar 13, 2016 at 16:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.