#14960 closed bug (fixed)

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

Reported by: Gabriel "_|Nix|_" Schulhof Owned by: Alexander Schmitz
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.

Change History (3)

comment:1 Changed 20 months ago by Alexander Schmitz

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

comment:2 Changed 20 months ago by Scott González

Milestone: none1.12.0
Priority: minorblocker
Status: newopen
Version: git (not yet released)1.12.0-rc.2

comment:3 Changed 19 months ago by Alexander Schmitz

Owner: set to Alexander Schmitz
Resolution: fixed
Status: openclosed

In 04b670e:

Controlgroup: Don't remove existing classes classes

unless its a corner class

Fixes #14960

Note: See TracTickets for help on using tickets.