Skip to content

Support for <base /> tag. #853

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

Closed
wants to merge 1 commit into from
Closed

Support for <base /> tag. #853

wants to merge 1 commit into from

Conversation

benlesh
Copy link

@benlesh benlesh commented Nov 29, 2012

Added data-localid="#idhere" functionality to tabs widget to better support pages using the tag. See Bug: http://bugs.jqueryui.com/ticket/8637

@jzaefferer
Copy link
Member

#8637 was closed as "notabug", see also http://bugs.jqueryui.com/ticket/7822

@jzaefferer jzaefferer closed this Nov 29, 2012
@benlesh
Copy link
Author

benlesh commented Nov 29, 2012

Did you even read what I did? Just provided a workaround method of registering local tabs.

@benlesh
Copy link
Author

benlesh commented Nov 29, 2012

Ah well... thanks for having a look, whether you did or didn't. It's hard being polite in the opensource world when you're so popular I suppose.

@scottgonzalez
Copy link
Member

@Blesh I'm sorry if you feel that @jzaefferer closing the ticket with a short and to the point response was impolite. The fact is that this has been beaten to death. We've made it abundantly clear that we will not land any hacks to make this work. Not only did you send a PR for something we specifically said we don't want, you took code that conformed to our coding standards and changed it so that it no longer conforms.

@benlesh
Copy link
Author

benlesh commented Nov 30, 2012

Scott, my apologies for not meeting your coding standards. For educational purposes... where are they listed so I might consider them if I ever do another pull request?

@scottgonzalez
Copy link
Member

Our coding standards are posted on our development and planning wiki: http://wiki.jqueryui.com/w/page/12137737/Coding%20standards

I'm sure you can understand our reason for not wanting this change; hopefully that won't deter you from sending future pull requests.

@benlesh
Copy link
Author

benlesh commented Nov 30, 2012

Not at all. I'll admit it was a little hackish. I didn't expect you to implement it I guess, but I was hoping it would help get gears turning so people stopped pestering you about it.

@benlesh
Copy link
Author

benlesh commented Nov 30, 2012

Last question... was this the coding standard that I violated?

  • Don't save any data on the element other than the plugin instance.
    • An exception to this rule is if you need to store data on subordinate elements.

That's a little vague, is that to mean "No data-whatever attributes"?

@scottgonzalez
Copy link
Member

No, the violations were against the main jQuery coding standards (spacing and double quotes). For example, $(anchor).data('localid') should be $( anchor ).data( "localid" ). The regression was on lines 403 and 404.

The rule about data is not to use .data() as a setter to store information against the DOM. Instead, the data should just be stored on the widget instance. You can see an example of the exception in autocomplete, where data is stored on generated suggestion items.

@benlesh
Copy link
Author

benlesh commented Nov 30, 2012

Okay. Thanks for the feedback. I'll keep that in mind if I ever submit another pull request.

@gnarf
Copy link
Member

gnarf commented Dec 1, 2012

I'm actually a fan of the spirit of this pull... data-panel="#localid" could make this problem go away forever. Anything with a specified panel could be forced to be a "local" tab, totally circumventing the <base> issue.

@YuriyHorobey
Copy link

What a nice discussion about coding standards...
The point is that I am for already TWO days trying to "hack" jQ-ui in such a way it will work.
The result is that I am starting (again, last time it was a year or so ago) to search for other jQ based UI library.
That's because I can't explain my manager why tabs are working on homepage (where location.href=='what is in base tab') and will not work on other pages. The guy just will not understand me. And the very first I'll hear "why don't you use another solutions?" I will event not try to start explaining him "jQ-ui is 'industry standard' 'rock solid', '..community..'"; just don't want them to laugh at me.
It is really great pity, that you react on peoples troubles in such a way.
Is that really so difficult to take code from blesh (non-conforming standards, 'its a hack'), quickly edit it to conform everything add to the library, release and hear lots of heartly thanks?
I hate to write comments like this.

@impressto
Copy link

Sure wish there was a note about this in the UI Tabs documentation and samples. I pulled my hair out trying to figure this one out. My solution was to simply remove the base tab but that is probably not an option in many cases.

If you are on an MVC system like CodeIgniter and need to keep the base tag, a quick fix is to use the REQUEST_URI variable.

Here is a code sample:

<?php $request_uri = getenv("REQUEST_URI"); ?>

<div id="Tabs">
<ul>
<li><a href="<?=$request_uri?>#page_settings">Page</a></li>
<li><a href="<?=$request_uri?>#page_content">Content</a></li>
</ul>

... CONTENT DIVS HERE ...

</div>

@scottgonzalez
Copy link
Member

What exactly should the documentation say?

@impressto
Copy link

It could mention this new behavior so people don't have to figure it out on their own ;) Just sayin!

@dasblauehandtuch
Copy link

I personally consider this entire issue as a bit user-unfriendly - you cannot simply "define" the reality away - and the reality sadly is, that this new behaviour of tabs causes massive problems (EDIT: if your site needs a base tag).

I understand your "wish" and "desire" for coding/markup standards, and I support it, but imo the jQuery team should not try to play god here, you should try to provide some backwards compatbility too - right now, I only can just suggest to everyone who considers to upgrade a large website from 1.8 to 1.9 "better let it be, if you have been using tabs." (EDIT: if your site needs a base tag)

@Blesh: Thanks a lot for your effort, even if it forces us into a patched jq-environment, it at least seems to solve the issue.

@scottgonzalez
Copy link
Member

...you cannot simply "define" the reality away...

I agree 100%! Please learn how <base> works.

@dasblauehandtuch
Copy link

See, I am not trying to lecture you, neither to blame you, I appreciate your work, really, but I think that you right now ask for too much - it is not about understanding the base tag - most of us understand it quite well :)

I am only trying to explain that there are a lot of elder applications out there requiring a base tag, and you will not find anyone being willing to pay the time it takes to update them - so our only choice is in these cases either to patch jqeury-ui or to stay on 1.8, thats both very unnecessary from my point of view.

But we will try your "prevent-default" suggestion at http://bugs.jqueryui.com/ticket/7822 , thank you thatfor a lot.

@angelcervera
Copy link

@dasblauehandtuch I completely agree with you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

8 participants