patternjavascriptMinor
Non-coder options in Javascript
Viewed 0 times
javascriptoptionscodernon
Problem
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:
I don't like all the
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
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
Solution
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
I believe this can be replaced by the following, removing the dependency to jQuery.
In the end, your code is about getting
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;Code Snippets
var pageTitle = $('meta[property="og:title"]').attr('content');var pageTitle = document.querySelector('meta[property="og:title"]').getAttribute('content');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;Context
StackExchange Code Review Q#122653, answer score: 3
Revisions (0)
No revisions yet.