[jira] [Comment Edited] (WICKET-4904) Use a behavior that modifies AbstractLink's markup when disabled

["Garret Wilson (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/WICKET-4904?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059918#comment-14059918 ] 

Garret Wilson edited comment on WICKET-4904 at 7/12/14 10:48 PM:
-----------------------------------------------------------------

This ticket is awesome --- it was exactly what I was raising on the dev list. I'm happy to see that others share my philosophy of having Wicket be hands-off (as much as possible) with the HTML (unless some modification is appropriate as per the standards, of course, such as removing the href).

I have some doubts about the actual implementation of {{DisabledLinkBehavior}}, though. I completely support the concept of adding a simple behavior to bring back pre-7.x behavior. But...

* I don't think the name truly reflects what the behavior does. In reality, this is a "bring back legacy functionality" behavior, not a general-purpose behavior for disabled links.
* Even using the constructor, I can't change the fact that it changes certain tags to {{<span>}}; that feature is hard-coded in.
* And I'm not sure I understand why certain tags (e.g. {{<a>}}, {{link}}) are hard-coded --- maybe I would want to add this behavior to non-links, using its before/after tag additions.
* Does the class need to be {{final}}? What if I want to extend it?

I guess I would recommend:

* Rename the class to something like {{DisabledComponentWrapperBehavior}}.
* Remove {{final}}.
* Add the tag name replacement as a third (optional) customization parameter in the constructor.
* Remove the hard-coded element names.
* Remove the default constructor.
* Add a factory method called {{DisabledComponentWrapperBehavior.createWicket6DisabledLinkBehavior()}} which would simply return {{new DisabledComponentWrapperBehavior("<em>", "</em>", "span")}}.

_Update:_ I also find the {{DisabledLinkBehavior.LinkInstantiationListener}} to be a little out of place and opaque as to its purpose. However, adding {{DisabledComponentWrapperBehavior.createWicket6DisabledLinkBehavior()}} to a {{Wicket6BehaviorBundle}} would be ideal. That way Wicket could be reverted to pre-7.x behavior in one swoop. See my {{BehaviorBundle}} proposal on the dev list.

I know I'm new here --- please don't take offense at the code analysis. I hope some of the suggestions are helpful.

In any case, I do have some suggestions about how to easily add disabled attributes for things like Bootstrap and Pure. I'll investigate further and report back.


was (Author: garretwilson):
This ticket is awesome --- it was exactly what I was raising on the dev list. I'm happy to see that others share my philosophy of having Wicket be hands-off (as much as possible) with the HTML (unless some modification is appropriate as per the standards, of course, such as removing the href).

I have some doubts about the actual implementation of {{DisabledLinkBehavior}}, though. I completely support the concept of adding a simple behavior to bring back pre-7.x behavior. But...

* I don't think the name truly reflects what the behavior does. In reality, this is a "bring back legacy functionality" behavior, not a general-purpose behavior for disabled links.
* Even using the constructor, I can't change the fact that it changes certain tags to {{<span>}}; that feature is hard-coded in.
* And I'm not sure I understand why certain tags (e.g. {{<a>}}, {{link}}) are hard-coded --- maybe I would want to add this behavior to non-links, using its before/after tag additions.
* Does the class need to be {{final}}? What if I want to extend it?

I guess I would recommend:

* Rename the class to something like {{DisabledComponentWrapperBehavior}}.
* Remove {{final}}.
* Add the tag name replacement as a third (optional) customization parameter in the constructor.
* Remove the hard-coded element names.
* Remove the default constructor.
* Add a factory method called {{DisabledComponentWrapperBehavior.createLegacyDisabledLinkBehavior()}} which would simply return {{new DisabledComponentWrapperBehavior("<em>", "</em>", "span")}}.

I know I'm new here --- please don't take offense at the code analysis. I hope some of the suggestions are helpful.

In any case, I do have some suggestions about how to easily add disabled attributes for things like Bootstrap and Pure. I'll investigate further and report back.

> Use a behavior that modifies AbstractLink's markup when disabled
> ----------------------------------------------------------------
>
>                 Key: WICKET-4904
>                 URL: https://issues.apache.org/jira/browse/WICKET-4904
>             Project: Wicket
>          Issue Type: Improvement
>          Components: wicket
>    Affects Versions: 6.3.0
>            Reporter: Martin Grigorov
>            Assignee: Sven Meier
>            Priority: Minor
>             Fix For: 7.0.0-M1
>
>         Attachments: WICKET-4904.patch
>
>
> Currently Wicket transforms <a> tag to <span><em> when a link is disabled.
> Recently Wicket-Bootstrap user filed issue https://github.com/l0rdn1kk0n/wicket-bootstrap/issues/79 that this breaks the UI because Twitter Bootstrap needs just 'disabled="disabled"' attribute in <a>.
> With the new strategy it will be possible to set global strategy and override it per link.



--
This message was sent by Atlassian JIRA
(v6.2#6252)