Why yii\grid\ActionColumn use TwitterBootstrap dependent markup

[eresik]

yii\grid\ActionColumn use TwitterBootstrap dependent markup by default.

It's not very good.

It turns out that you are not using the bootstrap, and the module uses ActionColumn, you have to take advantage of dependency injection for that would see the standard actions buttons.

https://github.com/yiisoft/yii2/blob/master/framework/grid/ActionColumn.php#L143
https://github.com/yiisoft/yii2/blob/master/framework/grid/ActionColumn.php#L153
https://github.com/yiisoft/yii2/blob/master/framework/grid/ActionColumn.php#L165

[samdark]

What do you propose to use instead of it?

[SilverFire]

I agree that it's not a good thing.
We can create an ActionColumn class in bootstrap extension and define those bootstrap-dependent buttons there.

[samdark]

@SilverFire OK but what do you propose to use for non-bootstrap buttons then? Also it would likely break existing apps.

[SilverFire]

Anyway it's a BC breaking change that goes to 2.1.
Non-bootstrap buttons will look ugly :)

[dynasource]

moving anything external out of the core software is always a good idea.
If you do it right, why would it break BC?

Why not compare icons with the logic behind translations.
Why not give create a default icon mapping and the user the option to customize/overwrite this "icon-mapping"

[samdark]

Because in existing apps bootstrap buttons would be replaced with something else on update.

[dynasource]

it wouldnt if you make the icon mapping dependant on the version of TWBS

[samdark]

How's that?

[dynasource]

you define:

• a core Icon class
• in every view, for every icon, you call this Icon class
• based on the 'icon library', you define its root path
• based on the version of this library, you define the path of the mapping file
• with every update of the Icon library, you refer to the correct mapping.

[samdark]

OK. Two questions:

1. What would it use if there's no bootstrap extension present?
2. How not to break compatibility to for existing users with bootstrap extension used?

[dynasource]

1. if no icon library is used, the base Icon class wont render an Icon
2. if you use TWBS for existing users
   /web/Icon.php
   /web/Icon::$libraryPath
   /web/Icon::$version
   /web/Icon::render('update')

and then a folder:
/web/icon/twbs/1_0.php
/web/icon/twbs/2_0.php
/web/icon/twbs/3_0.php

With ie. 2_0.php:

return [
'update'=>'glyphicon-ok-2'
]
With ie. 3_0.php:

return [
'update'=>'glyphicon-ok-circle'
]

[samdark]

How would Icon detect if twbs is installed?

[dynasource]

well, you provide a default path for the mapping file in Icon, like:
'@vendor/yiisoft/yii2-bootstrap/mapping.php

If the file does not exist, nothing is rendered.

You have to put a default somewhere, as you want to have a default theme anyways.

[samdark]

So for all existing apps there will be a need to specify additional path else icons will disappear. That's what I call backwards compatibility break.

[dynasource]

If people update to 2.0.7, they just like now:

• have a default reference to glyphicons,
• except this is done centrally customizable in /web/Icon.php by a connector-label, like 'update'
• if TWBS is installed, the corresponding icon will be shown
• if not installed, the connectorlabel will be shown
• if you have your own library, create the mapping file and off you go.

Nobody would have to get any issue.

[samdark]

Yes, that would solve it.

[samdark]

It's still BC breaking since the way of configuring it would change, right?

[dynasource]

whether you are a hardcoding it in the view with names like 'glyphicons'
or using a mapping with 'connectors' wont change anything for current users, right?

[samdark]

Right.

[dynasource]

I have a solution ready. Will create a PR next week.

[samdark]

Won't be changed in 2.0.