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

Added option to specify target attribute for link menus #845

Closed
wants to merge 2 commits into from
Closed

Added option to specify target attribute for link menus #845

wants to merge 2 commits into from

Conversation

maldoinc
Copy link
Contributor

While a very welcome feature the new link menus imo lack the ability to open them in a new tab. This is especially useful for external links where you do not want the user to leave your application.

While opening new tabs is still possible via scroll click, ctrl + click etc this patch makes it possible to make that the default behaviour.

A new target property can be set on the link menus which then will be passed on to the generated anchor tag.

Possible values: _blank|_self|_parent|_top|framename where _blank is by far the most useful one.

@javiereguiluz please tell me how to go about adding tests for this feature.

Useful when wanting to open a link in a new tab where you simply set the `target` key to `_blank`
Added option to add target attribute to menu links
@Pierstoval
Copy link
Contributor

I like the need for adding new features specific to each link, but I prefer having an attr option, like we have in form fields. This would be much more flexible, and plus it would allow adding classes or other cool things.

@javiereguiluz
Copy link
Collaborator

@maldoinc thanks for proposing this feature. I agree with it and for now I don't agree with @Pierstoval's proposal to generalize this and allow to define more options (let's add new options one by one).

I've created #907 to make some tweaks to your pull requests:

  1. I've updated the tests, which is a bit tricky to do in our case.
  2. I've removed the restriction that make this feature only work for link type items. Now it works for any menu item.
  3. I've added a brief explanation in the docs about this new feature.

So I'm closing this pull request in favor of the newer one. In any case, I've reused your original commits, so you'll get full credit when merging this. Thanks!

javiereguiluz added a commit that referenced this pull request Feb 20, 2016
…for the menu links (maldoinc, javiereguiluz)

This PR was merged into the master branch.

Discussion
----------

Added a "target" option to specify the target attribute for the menu links

This pull request finishes the work done originally by @maldoinc in #845.

Commits
-------

5a00dae Added tests for the feature
13c1e58 Fixed tests
84584b3 Tweaked the new feature
af018e0 Merge pull request #1 from maldoinc/maldoinc-patch-menu-target
142531f Added option to add target attribute to menu links
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants