anchorjs icon indicating copy to clipboard operation
anchorjs copied to clipboard

Expose `urlify`

Open Kikobeats opened this issue 6 years ago • 2 comments

Hello,

I extracted the urlify method out of the constructor scope since it can be used without using Anchors directly.

That's is useful when you are working with server-side application and you want to associate a slug with your HTML elements titles.

The point is that you are interested in generating the same slug to later use AnchorJS in the frontend and be possible link the things properly.

Kikobeats avatar Jan 04 '19 16:01 Kikobeats

Hey @Kikobeats, thanks for the PR.

I'm on-board with using urlify on it's own. It's not documented, but you can already do this with anchors.urlify() like I do in the tests.

What's the benefit of pulling it out of the constructor? Are you thinking we can avoid instantiating an Anchors object this way? Something like this:

// Before
var anchorJS = require('anchor-js');
var anchors = new anchorJS();
var idText = anchors.urlify('My Example Heading');

// After
var anchorJS = require('anchor-js');
var idText = anchorJS.urlify('My Example Heading');

I could be on board with this as long as we don't have to make too many compromises. What you have in your PR is a good start but I see a couple issues:

  1. It looks like the _applyRemainingDefaultOptions function probably can't be called in the scope you moved urlify to. This might require a bigger refactor.
  2. It looks like we have some failing linting rules and probably tests too.

Feel free to take another shot at these issues if you'd like. If you'd prefer to simply call urlify from the anchors object, that works for me too.

bryanbraun avatar Jan 05 '19 03:01 bryanbraun

That's exactly the in intention: it looks I need to spend a bit more time in extract the method properly 🙂

Kikobeats avatar Jan 05 '19 08:01 Kikobeats

Note: this branch was automatically closed when I switched the default development branch from master to main. If you want to revisit this feature, feel free to open a new pull request against main.

bryanbraun avatar Jan 18 '23 15:01 bryanbraun