Allow for external links in the menu

[sebastien-fauvel]

No description provided.

[sebastien-fauvel]

@rabmyself I think it should be a possible use case that some of the menu entries (say, the last one) are actually external links.

You get a jQuery exception Syntax error, unrecognized expression: . in this case, because the selector matchingSection = $("."+id) is invalid if the id attribute is missing.

So I'm just filtering out menu links without an id attribute now. I think it should be fine? Would you mind merging this?

---

And what about the demo page? Would you like to update the script (and its minified version) in this place, too?

[willfaulds]

I understand what you're trying todo and why.

but making smint completely reliant on ids is not a great idea (ruins progressive enhancement). We should be able to use an anchor hash instead of an id. Infact this should be the promoted method.

Perhaps it would be better to add logic which tests to see if a DOM object can be found and if not then disregard the link?

[sebastien-fauvel]

You mean the goal should be to set:

var matchingSection = $('a[name="'+hash+'"]')

whenever possible, only consider the id as a fallback, and ignore the menu link if both hash and id are missing? Ok, makes sense... I'm going to do it this way, if that's what you mean.

[willfaulds]

If I understand you correctly. I was thinking earlier at the point of initialization.

This is for example only -

        $smintItems = $([]);

        $smint.find('a').each(function() {              
            if( $(this).prop( "hash" ) ){
                $smintItems.push( $(this) );
            } else if ( $(this).attr('id') ){
                $smintItems.push( $(this) );
            }
        }),

so only adding found to the $smintItem list if their href contains an anchor (hash) or the has an id...

Even better would be to explicitly check if a DOM element actually exists before adding them to the $smintItem list. Therefore reducing the chance of errors.

[sebastien-fauvel]

I've just seen that there is a class smint-disableAll for external menu links, which is basically what I've been looking for. So this issue could actually be closed as far as I am concerned.

---

About your proposition:

• Filtering out menu links without a hash and without an id is definitely a good idea, since they cannot be mapped to any matchingSection to scroll to.
• For backward compatibility, we should also filter out links with the class smint-disableAll. Not sure about the class smint-disable? What it is for actually?
• We should definitely check whether the matchingSection can be found in the DOM, too. Otherwise you get exceptions like "Cannot find property top of undefined".
• We should look for an anchor with the given hash for each menu link, and use it as the matchingSection in priority. This way smint would just improve the standard behaviour, which is to spring to this anchor. Looking for a matchingSection with the same class name as the menu link's id is rather a hack, and should only be a deprecated fallback imo.