Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bootstrap Anchor plugin #16248

Closed
markhalliwell opened this issue Apr 7, 2015 · 7 comments
Closed

Bootstrap Anchor plugin #16248

markhalliwell opened this issue Apr 7, 2015 · 7 comments

Comments

@markhalliwell
Copy link

Hello all.

I'd first like to preface this with a little about myself. I am the primary maintainer for the Drupal Bootstrap base-theme. It currently has over 60k installs, so needless to say we get a lot of our own issues.

Recently, I started the task of trying to tackle better anchor and scrolling support in the base-theme. Support for anchors or scrolling to them has always been a big pain, for not only me, but also a lot of others; especially when dealing with fixed navbars [jsbin].

The code I currently have in the project was rather hack-ish and completely un-configurable (my feeble attempt to satisfy a growing issue), so I set out to tackle this once and for all [https://www.drupal.org/node/2462645]. I've poured over countless blogs, docs, support forums and even found a few in this project's issue queue [#193, #1768, #11854].

The "solutions" vary from CSS to JS, but more often than not require at least some sort of manual configuration for it to work with that specific theme/offset requirement.

Ultimately though, I've determined that a CSS-only solution just doesn't really cut it; especially when dealing with dynamic and varying content as such in CMS framework like Drupal. So, I've created the following JS based plugin instead:

https://github.com/markcarver/bootstrap-anchor [demo/docs]

In an effort to offset the "cost" for using JS, I was also inspired with the recent addition using anchor.js for Bootstrap's own documentation (#14897, #15491). This got me to thinking how this could easily be expanded from Bootstrap's own Tooltip plugin, like Popovers are. What makes it even more enticing is that there has been a glyphicon-link available to use for quite a while.

Since both Tooltips and Popovers are completely opt-in with no data API support, it made sense to follow in that direction; especially for those who still want/need a CSS-only solution.

My ultimate goal/desire is to merge this upstream into Bootstrap directly. I'm sure this will be a daunting task which is why I'm creating this issue now to get some feedback and eyes on this plugin so it can be flushed out a bit more. I'm still working on documentation of all the new options, so bare that in mind.

I look forward to y'alls feedback!

@cvrebert
Copy link
Collaborator

cvrebert commented Apr 8, 2015

I personally doubt the Core Team would be interested in getting into the same business as Anchor.js when Anchor.js itself already does a fine job.

As for #1768 & friends, you might have more traction on that front, although we're fairly reluctant to add more jQuery plugins at this point. I too wish there was a good solution to that bug though.

@markhalliwell
Copy link
Author

same business as Anchor.js

Except that Anchor.js isn't bundled with Bootstrap. This is about providing and out-of-the-box native Bootstrap plugin... not relying on yet another 3rd party external library that people will have to download, install and configure in addition to.

Anchor.js itself already does a fine job

That's certainly debatable. It's lightweight and works well for simple sites, sure, but also at the cost of being easily configurable in case complexities arise.

It doesn't use a template for inserting existing markup that a framework (like Bootstrap) already has available and the only way to replace the "icon" is to override/replace the CSS it provides. It's ID generation logic could also use some work.

I'm really not trying to knock it, in fact it was in part responsible for the discovery process that led to this plugin. It certainly has it's use cases, but I wouldn't have built what I did if it did a "good" enough job.

I would like to make it clear that this plugin is a pure extension on top of the existing Bootstrap tooltip plugin. It is and was not an Anchor.js rip-off, just inspired by it as well as Bootstrap's existing Tooltip plugin.

As for #1768 & friends, you might have more traction on that front

I think you're missing the point of the plugin. It's goal is to really tie two concepts together: anchors and links to anchors (anchor links). By doing so, they're aware of each other and can work together in harmony to provide a more cohesive anchor/scrolling solution.

although we're fairly reluctant to add more jQuery plugins at this point

Sure. I'm not suggesting that this should be in the next release. It's currently not, by any means, "production" ready. In fact, I imagined that it would be BS4 in all reality (with the separate repo for BS3 support/prototyping).

Like I said above, I opened this issue for exposure and to get viable feedback. I was hoping to have more of a positive feedback and to move forward rather than defending a solution to a problem that has been quite evident for quite some time now.

I'm certainly open to suggestions, improvements and even changing some fundamental aspects about the plugin, it's not set in stone. I urge people to seriously consider this proposal rather than simply dismissing it outright because of "simpler" existing plugins.

@cvrebert
Copy link
Collaborator

cvrebert commented Apr 8, 2015

Except that Anchor.js isn't bundled with Bootstrap. This is about providing an out-of-the-box native Bootstrap plugin

Just speaking generally: There is a limit to what we can bundle, both due to the amount of effort required (e.g. I'm doubtful that Bootstrap itself will ever include a datepicker; they're just too damn complex and we already have our work cut out for us just maintaining the existing widgets) and due to fears of bloat (which we're already occasionally accused of with just our current set of widgets).
And the trend these days is towards smaller modular packages rather than kitchen sinks.
Actually, one of the hopes for Bootstrap v3 was to strengthen the third-party plugin ecosystem and thus lessen the pressure for new plugins in core Bootstrap, but unfortunately not much happened on that front.

Anyway, I'll be interested to see what the rest of the team says about the proposal.

@markhalliwell
Copy link
Author

I'm rather curious as to the hesitation. Have you even looked at the code and demo yet? I've already done most of the ground work...

https://github.com/markcarver/bootstrap-anchor
http://markcarver.github.io/bootstrap-anchor/

I'm doubtful that Bootstrap itself will ever include a datepicker

I agree, but this isn't about just adding a arbitrary feature, it's also a solution to an issue that this framework introduces in the first place by providing a fixed navbar.

It's kind of like saying:
"Here's a nifty feature for navigation, but don't mind that your anchors may behave a bit oddly at times. Will we fix it, no... there's a few hacks around the interwebs, do some research."

we already have our work cut out for us just maintaining the existing widgets

I will gladly accept sole responsibility for maintaining this specific plugin it if it were ever to come to that. Somehow though, I doubt that will happen considering it's already built upon and in the same format as the Tooltip plugin.

Anyway, I'll be interested to see what the rest of the team says about the proposal.

Yes, I would be too.

@zacechola
Copy link

@MarkCarver A few things:

It's interesting to me that you get a lot of requests for the feature in your Drupal theme. Forgive me, but it's been quite some time since I touched PHP or any CMS but is that a common sort of feature in Drupal themes? I haven't seen it come up too often in Bootstrap's IRC channel or elsewhere, so it's interesting to me that it'd be a requested feature elsewhere.

I'd also be interested if it's the sort of plugin other theme developers might want to use.

Secondly, this is a bit unfair:

Here's a nifty feature for navigation, but don't mind that your anchors may behave a bit oddly at times. Will we fix it, no... there's a few hacks around the interwebs, do some research."

The docs note that using fixed navbars requires some padding adjustments. While that doesn't cover every combination of components and HTML on a page (a fixed navbar and anchors to IDs in this specific instance). Solving that problem is usually done simply with an offset on the click.

Bootstrap-anchor does much more than solve that one issue, though, which brings me to my third thing. Adding features, especially full-fledged plugins, need to be evaluated in terms of long-term maintenance costs (it's great that you're offering to support it, but it's also no guarantee, unfortunately), added complexity (you'll be surprised by how authors use Bootstrap in unexpected ways) and package costs (like @cvrebert pointed out, Bootstrap already gets flack for being heavy in many ways).

Anyway, I'm not saying 👎 yet but I'm not sold yet on why this should be a core plugin and not a community plugin.

@markhalliwell
Copy link
Author

I touched PHP or any CMS but is that a common sort of feature in Drupal themes?

I wouldn't say it's "common" per se, but it certainly more of a prevalent issue when Drupal modules like the Administration Menu add their own fixed nav and increases the body margin (not padding) on top of a user who has opted to use the Bootstrap fixed navbar that adds body padding; it can get rather complicated and nasty very quickly.

I haven't seen it come up too often in Bootstrap's IRC channel or elsewhere, so it's interesting to me that it'd be a requested feature elsewhere.

I think that's because people who are in the business of starting with Bootstrap directly (i.e. reading the docs, in the IRC channel) are generally themers/site builders or at the very least able to understand that there will be a minimal amount of effort in configuring their "theme".

The Drupal Bootstrap base-theme's sole purpose is to bridge that gap between the dynamic aspect of Drupal and the Bootstrap framework. One of the main reasons this base-theme has risen to become very popular (currently third top Drupal theme) in such a short amount of time is because it allows non-themers to configure their Bootstrap theme in an easy to use administrative UI.

I understand that I come from a unique perspective and it's partially a unique issue to us. Users choose Drupal Bootstrap because the framework is popular. The don't always choose it because they know the little intricacies/issues that come with choosing their fixed navbar type from the UI:

screen shot 2015-04-08 at 2 10 32 pm

These types of users are not always skilled CSS/JS ninjas, so when they see that there's an issue with their anchor's "Solving that problem is usually done simply with an offset on the click" is really not always as "simple" as it sounds. Which is why I've been working on a way to assist with/fix this problem.

Bootstrap-anchor does much more than solve that one issue, though

Yes, I understand and acknowledged this fact up front as one of the reasons for offsetting the "cost" of using a JS based solution to the scrolling issue by expanding upon Bootstrap's Tooltip plugin with anchor links.

Also, again, it goes further than just "adding in another feature" really. The two work in together in harmony. By building the plugin with the concept of "anchors" and "anchor links", it allows for a more cohesive UX in addressing "What happens when I click an anchor on this page?".

I do find it a little ironic though that even the docs suddenly had a need for them, but there wasn't anything native in Bootstrap to do it so a third party plugin was used instead.

Adding features, especially full-fledged plugins, need to be evaluated in terms of long-term maintenance costs (it's great that you're offering to support it, but it's also no guarantee, unfortunately), added complexity (you'll be surprised by how authors use Bootstrap in unexpected ways) and package costs (like @cvrebert pointed out, Bootstrap already gets flack for being heavy in many ways).

I really do understand all of this and agree with it, believe me, I do. I'm not suggesting this plugin is just thrown in willy nilly without any sort of evaluation, game plan or what have you. I think the real purpose of why I opened this issue in the first place is really being lost here.

I'm not sold yet on why this should be a core plugin and not a community plugin.

I'm not saying it can't be either. I opened this issue to get evaluation and feedback on the plugin itself, not quarrel over semantics. Yes, I said it was my goal/desire to ultimately make this a core plugin, but who knows, maybe that won't be the case and I'll just keep it as a separate contrib plugin. That is certainly fine by me.

All I was looking for really was "Wow that's cool... let's work on making it better! Maybe it's something that does have merit to pull into core."... you know FOSS, not getting smacked down for having an idea on how to fix a very real issue.

I'm honestly half tempted to just close this issue and just continue developing it as a contrib plugin if this is going to be the attitude moving forward 😦

@mdo
Copy link
Member

mdo commented Apr 13, 2015

Your plugin looks and feels awesome in my super quick tests, so props on that front. I'll try to address as much of your post and the replies here as I can.

Development approach

Since both Tooltips and Popovers are completely opt-in with no data API support, it made sense to follow in that direction; especially for those who still want/need a CSS-only solution.

Tooltips and popovers are opt-in do to performance issues with detecting :hover, :focus, etc on every DOM element. Good call here, that does indeed make sense.

Your goal and feedback

My ultimate goal/desire is to merge this upstream into Bootstrap directly. I'm sure this will be a daunting task which is why I'm creating this issue now to get some feedback and eyes on this plugin so it can be flushed out a bit more.

Right on, a bold endeavor to say the least :). I think @cvrebert, as a core team member, covered the generalities quite well, though I think you've mistaken some of his comments as overly dismissive. He's outlined that:

  1. We're reluctant to add new JavaScript. Expanding on that, we're not adding any new features to v3. It's a call we made to avoid having to rewrite even more for v4.
  2. We've got Anchor.js in there right now and it's solving the problem it was meant to solve—linking to specific sections of content—quite well.
  3. There's a balance to what we can do with the size of the team we have. We build things we think folks would like to use, as well as things we ourselves want to build and maintain.

You didn't get any real feedback on your own plugin, but that's not really something we're here to provide unfortunately. At least, not in a structured or timely way that would be of much use to yourself. Looking at the demo and docs though, it seems to work quite well and is a decently structured and focused plugin. All around, it seems great.

Fixed navbars

Yes, we've got fixed navbars and yes there's the problem of overlapping content. I'd prefer we didn't add more JavaScript to address that, but I understand folks also don't want to have to address these kind of things themselves. I'm torn on how far to go there, and because of that, we likely won't try to address it ourselves. I'd like us to continue to err on the side of caution for this kind of stuff.

Maintenance

You've offered to maintain it, but that's not enough. Putting it in the main repo means committing to it and supporting it as a team. Bug reports, further feature development, and documentation all come into play there. I'm not saying you wouldn't be up to it, but we might not be. That's not meant to be dick-ish, it's just matter-of-fact.

On feedback

All I was looking for really was "Wow that's cool... let's work on making it better! Maybe it's something that does have merit to pull into core."... you know FOSS, not getting smacked down for having an idea on how to fix a very real issue.

I'm honestly half tempted to just close this issue...

No one has smacked you down. One team contributor and one community member voiced their opinions. Their questions and comments are just as valid as your own. While you've very clearly asked for feedback, you also very clearly stated you'd like this to be part of the core project. Two folks have focused on the latter while reviewing this thread.


Lastly, shifting gears back to the plugin itself, it's a damned fine plugin. I'd like to see it continue as a community project as I don't think it's something we'd like to see addressed with Bootstrap. Yes, the fixed navbar introduces a potential need for this. However, that's not unique to our navbars—damned near every component likely introduces a potential need for n number of edge cases, extensions, etc.

Please do continue to develop and share it with folks, but it won't be part of Bootstrap's core (not in v3 or v4, not at this point at least). Please also continue to use this thread to gather feedback from folks if you wish. I'd love if more of our team chimed in, but we're all on different schedules and what not.

Hope that helps some.

<3

@mdo mdo closed this as completed Apr 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants