Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ngTransclude content by default #11839

Closed
wants to merge 6 commits into from

Conversation

Projects
None yet
8 participants
@dmaslov
Copy link
Contributor

commented May 8, 2015

Existing content of the element that this directive is placed on will be removed only if transcluded content exists. In other case the content will be saved and used as content by default.

It will be useful if you need to use default content in directive's template and replace it only in some directive instances.

Example:

Directive template:
<button>
    <span ng-transclude>
        <i class="fa fa-paperclip"></i>Add to dashboard
    </span>
  </button>
Directive instance with modified content:
<!-- modified button content -->
<pin-chart-button>
    <i class="fa fa-thumb-tack"></i>
</pin-chart-button>
Which would generate:
<button>
    <span>
        <i class="fa fa-thumb-tack"></i>
    </span>
  </button>
Directive instance without modified content:
<!-- default button content -->
<pin-chart-button></pin-chart-button>
Which would generate:
<button>
    <span>
        <i class="fa fa-paperclip"></i>Add to dashboard
    </span>
  </button>
ngTransclude: Existing content of the element that this directive is …
…placed on will be removed if transcluded content exists. In other case the content will be saved and used as content by default.
@googlebot

This comment has been minimized.

Copy link

commented May 8, 2015

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@googlebot googlebot added the cla: no label May 8, 2015

@dmaslov

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2015

I signed it!

@googlebot

This comment has been minimized.

Copy link

commented May 8, 2015

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels May 8, 2015

@lgalfaso

This comment has been minimized.

Copy link
Member

commented May 10, 2015

Hi, would it be possible for you to:

  • Make it clear what is the use case
  • Create the tests that show the new behavior
  • Make the documentation changes of the feature

I am not very happy in changing the existing (already complex) transclude behavior, but with a good reason I can change my mind

@dmaslov

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2015

Use case:

Just imagine we have a case when we need to use a majority of identic elements throughout the project, for example buttons with the same text, look and behavior. Also there will be elements with the same behaviour but different look and content (e.g. text).

angular.module('transcludeDefaultContentExample', [])
.directive('myButton', function(){
            return {
              restrict: 'E',
              transclude: true,
              scope: true,
              template: '<button style="cursor: pointer;">' +
                          '<ng-transclude></ng-transclude>'
                        '</button>'
            };
        });

Now, having the current ngTransclude implementation, we need to duplicate the content of the ng-transclude element. And if there are many of such elements, we need to copy and paste the same content multiple times.

For example:

<my-button><b style="color: red;">Warning</b></my-button>
<my-button><b style="color: red;">Warning</b></my-button>
<my-button><b style="color: red;">Warning</b></my-button>
<my-button><b style="color: red;">Warning</b></my-button>

And only in case of elements with the different look and content we will type in another content.

<my-button><b style="color: green;">Ok</b></my-button>

The update I offer will let us avoid this multiple and useless copying & pasting. We will be able to set the ‘default’ look just once (as content of ng-transclude element) and change it only for the element that needs to look different.

angular.module('transcludeDefaultContentExample', [])
.directive('myButton', function(){
            return {
              restrict: 'E',
              transclude: true,
              scope: true,
              template: '<button style="cursor: pointer;">' +
                          '<ng-transclude>' +
                            '<b style="color: red;">Warning</b>' +
                          '</ng-transclude>' +
                        '</button>'
            };
        });
<my-button></my-button>
<my-button></my-button>
<my-button></my-button>
<my-button></my-button>
<my-button><b style="color: green;">Ok</b></my-button>
@dmaslov

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2015

Hi, @lgalfaso

  • Make it clear what is the use case
  • Create the tests that show the new behavior
  • Make the documentation changes of the feature
@dmaslov

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2015

related issue #7195

@dmaslov

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2015

Hey!
@Narretz is something wrong with my PR? Or it's just bad idea to do what i did? :)

@PavloKovalov

This comment has been minimized.

Copy link

commented Aug 25, 2015

@dmaslov @Narretz sound reasonable 👍

@DrCool

This comment has been minimized.

Copy link

commented Sep 29, 2015

I like this feature a lot. You've got my vote!

@Narretz Narretz modified the milestones: 1.5.x - migration-facilitation, Purgatory Nov 3, 2015

@Narretz

This comment has been minimized.

Copy link
Contributor

commented Nov 3, 2015

It does sound useful. Features for 1.5.x are closed, but we can put it on the table for 1.6 (it needs a rebase, though).

Merge remote-tracking branch 'upstream/master'
# Conflicts:
#	src/ng/directive/ngTransclude.js
@dmaslov

This comment has been minimized.

Copy link
Contributor Author

commented Nov 4, 2015

@Narretz, done. Could you please move it to 1.6.x milestone?

@pauljoey

This comment has been minimized.

Copy link

commented Nov 13, 2015

+1

@petebacondarwin petebacondarwin modified the milestones: 1.6.x, 1.5.x - migration-facilitation Nov 23, 2015

Merge remote-tracking branch 'upstream/master'
# Conflicts:
#	src/ng/directive/ngTransclude.js

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Dec 4, 2015

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Dec 4, 2015

@jniles jniles referenced this pull request Apr 25, 2016

Closed

Improve bhLoadingButton #243

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.