Skip to content

Conversation

pmaruszczyk
Copy link
Contributor

Fix for http://bugs.jqueryui.com/ticket/5253.

How can I change owner of ticket in bugtracker?

@dcneiner
Copy link
Member

I am a little concerned about the zoom: 1 in the jquery.ui.theme.css file. Most headers are block level by default, and if they are not, they can be handled on a per case basis. I feel like this change should have been made in the demo file where the .ui-widget-header is arbitrarily added to the span#toolbar.

Also, on the toolbar.html changes, the CSS hack is targeting only IE7, whereas both IE6 and IE7 could benefit from the change. The following change, combined with your inline-block change in jquery.ui.button.css, seems to achieve the same effect, but work in IE6 and IE7:

#toolbar {
  padding: 11px 4px 9px 4px;
  display: inline-block;
  *padding: 4px 0px 4px 5px;
}

@pmaruszczyk
Copy link
Contributor Author

Hi, about toolbar.html changes: I don't have ie6 so I coudn't test it. If you say that this also works in ie6 I can change it.

zoom:1 : Lets say I am noob user, I know only basic css styles. I saw nice toolbar demo on jquery-ui site, so I try make something similar on my site. I'm using jquery-ui, but wait! Why on ie7 it not looks like on demo site?! (What if I have not the toolbar.html file from some reason?) . I will never found how to fix it.

I think this souldn't be separated to case and framework/library. Jquery-ui is one thing, the specific case is another.

I don't test the zoom:1 in other examples, however I'm sure it wouldn't make another bugs.

@scottgonzalez
Copy link
Member

If users copy the markup from a demo, but not the styles, that's not something we should be concerned with. The fact that the toolbar is an inline element and causes layout issues is specific to the demo.

@pmaruszczyk
Copy link
Contributor Author

I will test my solution with other demos. I think if it will not destroy anything, it could be added to jquery.ui.themes.css. Because why not? However you manage the code, so I will relay on you.

@pmaruszczyk
Copy link
Contributor Author

I made second fix for this one, so this I'm closing.

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.

3 participants