Skip to main content

Search and Top Navigation

#14960 closed bug (fixed)

Opened May 02, 2016 07:04PM UTC

Closed May 26, 2016 01:16PM UTC

Controlgroup: Child widget classes unrelated to controlgroup position are removed because they share a class key

Reported by: gabrielschulhof Owned by: arschmitz
Priority: blocker Milestone: 1.12.0
Component: ui.controlgroup Version: 1.12.0-rc.2
Keywords: Cc:
Blocked by: Blocking:
Description

From a conversation with arschmitz:


gabrielschulhof:

While working on selectmenu, I ran into the following shortcoming of the classes option:

When you integrate a widget with controlgroup, you create function _widgetnameOptions() which returns the options that the widget is supposed to have in case it's a first/middle/last widget. Now, if you use class keys for this option, then the class key that you use is effectively unavailable for devs, because whenever you place this widget in a controlgroup the controlgroup is going to obliterate the value of the class key with the value you specify in _widgetnameOptions(). So, widgets that have generated elements that are supposed to be affected by the controlgroup (such as mobile selectmenu, for example), need to have two class keys referring to the element that's supposed to get the corner classes, for example. One for use by the controlgroup and the other for use by developers.

Otherwise you can't, for example, add an initially hidden widget to the controlgroup. Why not? Because you add

$( "<div>" )
  .appendTo( theControlgroup )
  .myWidget( { classes: { "tmp-mywidget" : "ui-screen-hidden" } } );
theControlgroup.refresh();

At this point the controlgroup will replace ui-screen-hidden with ui-corner-top, ui-corner-bottom, or nothing at all, but it will remove ui-screen-hidden, because it keys off the same class (tmp-mywidget). And you can't just add the attribute class="ui-screen-hidden" anywhere, because the div that would cause the whole widget to be hidden is a generated div - so the classes option is your only choice.

So, unless you add a second class key (tmp-mywidget-wrapper or tmp-mywidget-external-div or whatever) you can't add any classes to your generated div if it's also supposed to be part of a controlgroup.

Here's a bin illustrating the problem:

http://jsbin.com/yaduwuc/edit?html,output

... and here's one illustrating the solution (or workaround, depending on whether this is a bug or not):

http://jsbin.com/yuqisox/edit?html,output


arschmitz:

perhaps i should have it pass the instance here

https://github.com/jquery/jquery-ui/blob/master/ui/widgets/controlgroup.js#L240

The instance is already saved an available there can just make it pass

it so its available to those methods


gabrielschulhof:

That would definitely obviate the need for the extra class key.

Attachments (0)
Change History (3)

Changed May 02, 2016 08:03PM UTC by arschmitz comment:1

This is about adding new widgets non ui widgets to control group Ex the mobile selectmenu not current ui widgets

Changed May 04, 2016 04:32PM UTC by scottgonzalez comment:2

milestone: none1.12.0
priority: minorblocker
status: newopen
version: git (not yet released)1.12.0-rc.2

Changed May 26, 2016 01:16PM UTC by arschmitz comment:3

owner: → arschmitz
resolution: → fixed
status: openclosed

In [changeset:"04b670e6cc6b25ffe595c665ea86929f71f78b50" 04b670e]:

#!CommitTicketReference repository="" revision="04b670e6cc6b25ffe595c665ea86929f71f78b50"
Controlgroup: Don't remove existing classes classes

unless its a corner class

Fixes #14960