Skip to content
This repository has been archived by the owner on Apr 2, 2019. It is now read-only.

No Way (That I Can Tell) To Change Target Attribute on UriCells #242

Closed
machineghost opened this issue Jun 20, 2013 · 17 comments
Closed

No Way (That I Can Tell) To Change Target Attribute on UriCells #242

machineghost opened this issue Jun 20, 2013 · 17 comments
Milestone

Comments

@machineghost
Copy link

When you use UriCell to generate an <a> tag, that tag automatically comes with a target="_blank". While I understand I can write my own cell to get any target attribute I might want, this seems like an awfully opinionated behavior for the base library to have. Would it be possible to add some sort of option to UriCell to set the target attribute, or failing that an option to remove it entirely?

@kriswill
Copy link

You could override the render method to use a class property of target for the target attribute:

Backgrid.UriCell = Backgrid.UriCell.extend({
  target: '_blank',
  render: function() {
    Backgrid.UriCell.prototype.render.call(this)
    this.$('a').attr('target', this.target)
    return this
  }
})

@machineghost
Copy link
Author

Thanks @krisswill ... but as I noted "I understand I can write my own cell to get any target attribute I might want ...". It's not that a custom class isn't an option, as I'm happy to write such a sub-class if an in-library fix isn't viable. My point was more that BackGrid is a general purpose grid library, and that links that open in the same window have been around since the start of the web, so perhaps it would be an improvement to the library to support this use case instead of requiring custom code for it.

@kriswill
Copy link

I agree. There are many opinions that the default library has that aren't appropriate for certain use cases that I've encountered. I had to do a lot of sub-classing to tweak it's behavior. I would prefer that I didn't have to do it, but sometimes it can't be avoided.

If I were to speculate why it's like this, I think target='_blank' is a reasonable workaround, assuming you don't want to completely interrupt the Backbone application--since not having that would replace the current location. I have had to implement onbeforeunload handlers to prevent people from destroying their unsaved state when navigating away to static links, but this is very custom and messy and I wouldn't expect a Backbone app to have navigation counter-measures employed from a component like Backgrid.

If anything the UriCell probably should have some options passed thru from the column definition, that allows you to override the default target attribute of the link. How to do that in a tasteful and consistent way is the question.

@machineghost
Copy link
Author

If I were to speculate why it's like this, I think ...

Totally; it makes complete sense that BackGrid uses target="_blank" as its default, as that is certainly the "safest" option.

How to do that in a tasteful and consistent way is the question.

One simple way would be to change this line (from UriCell's render):

target: "_blank"

to:

target: this.target || "_blank"

This would allow anyone who wanted to have in-window links to simply define a UriCell sub-class ...

Backgrid.Extension.InWindowUriCell = Backgrid.UriCell.extend({target: ''});

... which they could then use just like UriCell:

new Backgrid.Grid({
    columns: [{cell: 'inWindowUriCell', ...

With just a one line change, I think you get a pretty tasteful/consistent improvement; after all, something similar is done when one overrides orderSeparator in an IntegerCell subclass.

@wyuenho
Copy link
Contributor

wyuenho commented Jun 21, 2013

The UriCell's defaults seem to be a constant nuisance for a lot of folks. In particular, currently people have to override render to change the behaviors of the title, target attributes and the innerText the anchor renders.

Would folks be satisfied if UriCell adopts a similar strategy as the NumberCell and DateTimeCell subclasses? That way you can extend a UriCell in-place inside the column definition to configure a custom UriCell.

@machineghost
Copy link
Author

I can't speak for anyone else, but as far as I'm concerned polymorphism is proven (both in Backgrid's NumberCell/DateTimeCell and just in programming in general), so it seems like the way to go with UriCell.

@wyuenho wyuenho mentioned this issue Jul 4, 2013
@kim2014
Copy link

kim2014 commented Mar 2, 2014

Sorry for the question but I would like to have the href different from title in UriCell. How to write the custom render to overwritten the backgrid's UriCell render. Again sorry I am very new to backgrid.

@machineghost
Copy link
Author

First off, a quick suggestion: if you have questions about how to use a library, a site like StackOverflow.com is much more geared towards that sort of thing than a Github issues comment thread. That being said, as I understand it ...

Backgrid uses the cell's original value as the URL, and it uses the cell's title property or the formatted value for the title. So, if your cell's contents are the URL, things are easy. You can either:

  1. (assuming your title and the link text are the same) make a formatter that "formats" the URL in to a title, or
  2. provide a title as a property of the cell (eg. override initialize and set a title in it).

If your cell's contents aren't the URL, then you do have to override the render method. Basically just do:

Backgrid.Cell.extend({
    // Copy/paste Backgrid's render in here
    render: function () {
        this.$el.empty();
        var rawValue = this.model.get(this.column.get("name"));
        var formattedValue = this.formatter.fromRaw(rawValue, this.model);
        this.$el.append($("<a>", {
          tabIndex: -1,
          href: rawValue,
          title: this.title || formattedValue,
          target: this.target
        }).text(formattedValue));
        this.delegateEvents();
        return this;
  }

And then replace the line:

              href: rawValue,

with whatever you want the URL to be.

Personally I would prefer if Backgrid used a lot of smaller functions so that overriding stuff like this was easier. Imagine if there was a method on Cell:

getHref: function(rawValue) {
    return rawValue;
}

and Backgrid used it instead of the rawValue you directly:

    this.$el.append($("<a>", {
          tabIndex: -1,
          href: this.getUrl(rawValue),

Then instead of having to copy/paste BackGrid code in to your code, you could just overwrite getUrl:

Backgrid.Cell.extend({
    getUrl: function(rawValue) {
         var myCalculatedUrl = doSomethingWith(rawValue);
         return myCalculatedUrl;
    })

... but it doesn't, so (as far as I can tell) you're stuck with the copy/paste.

@wyuenho
Copy link
Contributor

wyuenho commented Mar 2, 2014

I'll introduce more smaller functions into the cell classes. It looks like lots of people are confused.

@wyuenho
Copy link
Contributor

wyuenho commented Mar 2, 2014

I was just waiting for the API to get stabilized, looks like they are now.

@wyuenho
Copy link
Contributor

wyuenho commented Mar 2, 2014

Also, if you really would like to, here's a much simpler solution:

render: function () {
  this.__super__.render.apply(this, arguments):
  this.$('a').attr({'href': 'whateveruwant'});
  return this;
};

@kim2014
Copy link

kim2014 commented Mar 2, 2014

Thanks a lot Jeremy
My cell contents are not the URL(href) so I need to have the override render function as you mentioned below. Sorry for the basic question. Where I should place this override render function
Thanks
Kim

Sent from my iPhone

On Mar 2, 2014, at 10:41 AM, "Jeremy" <[email protected]mailto:[email protected]> wrote:

First off, a quick suggestion: if you have questions about how to use a library, a site like StackOverflow.comhttp://StackOverflow.com is much more geared towards that sort of thing than a Github issues comment thread. That being said, as I understand it ...

Backgrid uses the cell's original value as the URL, and it uses the cell's title property or the formatted value for the title. So, if your cell's contents are the URL, things are easy. You can either:

  1. (assuming your title and the link text are the same) make a formatter that "formats" the URL in to a title, or
  2. provide a title as a property of the cell (eg. override initialize and set a title in it).

If your cell's contents aren't the URL, then you do have to override the render method. Basically just do:

Backgrid.Cell.extend({
// Copy/paste Backgrid's render in here
render: function () {
this.$el.empty();
var rawValue = this.model.get(this.column.get("name"));
var formattedValue = this.formatter.fromRaw(rawValue, this.model);
this.$el.append($("", {
tabIndex: -1,
href: rawValue,
title: this.title || formattedValue,
target: this.target
}).text(formattedValue));
this.delegateEvents();
return this;
}

And then replace the line:

          href: rawValue,

with whatever you want the URL to be.

Personally I would prefer if Backgrid used a lot of smaller functions so that overriding stuff like this was easier. Imagine if there was a method on Cell:

getHref: function(rawValue) {
return rawValue;
}

and Backgrid used it instead of the rawValue you directly:

this.$el.append($("<a>", {
      tabIndex: -1,
      href: this.getUrl(rawValue),

Then instead of having to copy/paste BackGrid code in to your code, you could just overwrite getUrl:

Backgrid.Cell.extend({
getUrl: function(rawValue) {
var myCalculatedUrl = doSomethingWith(rawValue);
return myCalculatedUrl;
})

... but it doesn't, so (as far as I can tell) you're stuck with the copy/paste.

Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-36462139.

@kim2014
Copy link

kim2014 commented Mar 2, 2014

Should I have this render function in the model js.
Sorry where I should place this render function
Thanks
Kim

Sent from my iPhone

On Mar 2, 2014, at 11:02 AM, "Jimmy Yuen Ho Wong" <[email protected]mailto:[email protected]> wrote:

Also, if you really would like to, here's a much simpler solution:

render: function () {
this.super.render.apply(this, arguments):
this.$('a').attr({'href': 'whateveruwant'});
return this;
};

Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-36462772.

@machineghost
Copy link
Author

Exactly; however, I'd use @wyuenho's version instead of mine since his is a lot simpler.

@kim2014
Copy link

kim2014 commented Mar 3, 2014

Sorry for sending the email again.

I didn’t work. I added the subclass of UriCell to my model as so below and in my views I created the UriCell using subclass but got the error message:

Note: I had backbonejs’ version 1.1.0.

Uncaught TypeError: Object # has no method '_ensureElement' in backbone.js:990
Thanks
Jp

n This is view
createIfMapAccountGrid: function () {
this.model.fetch({reset: true});
var gridContainer = this.$el;

        var columns1 = [{
            name: "id", // The key of the model attribute
            label: "ORDER", // The name to display in the header
            editable: false, // By default every cell in a column is editable, but *ID* shouldn't be

            // Backgrid.Extension.SelectRowCell lets you select individual rows
            cell: "select-row",

            // Backgrid.Extension.SelectAllHeaderCell lets you select all the row on a page
            headerCell: "select-all"
        },  {

            name: "publisherId", // The key of the model attribute
            label: "Publisher Id", // The name to display in the header
            editable: false, // By default every cell in a column is editable, but *ID* shouldn't be
            cell:  App.Cells.CustomUriCell({
              orderSeparator: ''
            })
        }, {

n Model

(function () {
'use strict';
// Name space
window.App = {
Models: {},
Collections: {},
Views: {},
Cells: {}

};

App.Cells.CustomUriCell = Backgrid.UriCell.extend({
render: function () {
this.super.render.apply(this, arguments);
this.$("a").attr({href: "TestingURL"});
return this;
}
});

           // IfMapAccount model has `userId` and `publiserId` attributes.
 App.Models.IfMapAccount = Backbone.Model.extend({
    idAttribute: "userId",
    // Default attributes for the device
    defaults: {
         userId: '',
         publisherId: '',
         passwordHash: ''

    },

From: Jeremy [mailto:[email protected]]
Sent: Monday, March 03, 2014 8:16 AM
To: wyuenho/backgrid
Cc: Joanne Pham
Subject: Re: [backgrid] No Way (That I Can Tell) To Change Target Attribute on UriCells (#242)

Exactly; however, I'd use @wyuenhohttps://github.com/wyuenho's version instead of mine since his is a lot simpler.


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-36526016.

@kim2014
Copy link

kim2014 commented Mar 3, 2014

I replace the line:
cell: App.Cells.CustomUriCell({
orderSeparator: ''
})

To
cell: App.Cells.CustomUriCell
and Got the error message:
Uncaught TypeError: Cannot read property 'render' of undefined
And error is highlighted on ( this.super.render.apply(this, arguments);)

SubClass of UriCell

App.Cells.CustomUriCell = Backgrid.UriCell.extend({
render: function () {
this.super.render.apply(this, arguments);
this.$("a").attr({href: "TestingURL"});
return this;
}
});
Thanks
Jp

From: Joanne Pham
Sent: Monday, March 03, 2014 10:23 AM
To: 'wyuenho/backgrid'; wyuenho/backgrid
Subject: RE: [backgrid] No Way (That I Can Tell) To Change Target Attribute on UriCells (#242)

Sorry for sending the email again.

I didn’t work. I added the subclass of UriCell to my model as so below and in my views I created the UriCell using subclass but got the error message:

Note: I had backbonejs’ version 1.1.0.

Uncaught TypeError: Object # has no method '_ensureElement' in backbone.js:990
Thanks
Jp

n This is view
createIfMapAccountGrid: function () {
this.model.fetch({reset: true});
var gridContainer = this.$el;

        var columns1 = [{
            name: "id", // The key of the model attribute
            label: "ORDER", // The name to display in the header
            editable: false, // By default every cell in a column is editable, but *ID* shouldn't be

            // Backgrid.Extension.SelectRowCell lets you select individual rows
            cell: "select-row",

            // Backgrid.Extension.SelectAllHeaderCell lets you select all the row on a page
            headerCell: "select-all"
        },  {

            name: "publisherId", // The key of the model attribute
            label: "Publisher Id", // The name to display in the header
            editable: false, // By default every cell in a column is editable, but *ID* shouldn't be
            cell:  App.Cells.CustomUriCell({
              orderSeparator: ''
            })
        }, {

n Model

(function () {
'use strict';
// Name space
window.App = {
Models: {},
Collections: {},
Views: {},
Cells: {}

};

App.Cells.CustomUriCell = Backgrid.UriCell.extend({
render: function () {
this.super.render.apply(this, arguments);
this.$("a").attr({href: "TestingURL"});
return this;
}
});

           // IfMapAccount model has `userId` and `publiserId` attributes.
 App.Models.IfMapAccount = Backbone.Model.extend({
    idAttribute: "userId",
    // Default attributes for the device
    defaults: {
         userId: '',
         publisherId: '',
         passwordHash: ''

    },

From: Jeremy [mailto:[email protected]]
Sent: Monday, March 03, 2014 8:16 AM
To: wyuenho/backgrid
Cc: Joanne Pham
Subject: Re: [backgrid] No Way (That I Can Tell) To Change Target Attribute on UriCells (#242)

Exactly; however, I'd use @wyuenhohttps://github.com/wyuenho's version instead of mine since his is a lot simpler.


Reply to this email directly or view it on GitHubhttps://github.com//issues/242#issuecomment-36526016.

@wyuenho
Copy link
Contributor

wyuenho commented Mar 3, 2014

I just answered your ticket. Please stop cross-posting all over the internet.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants