-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add a RowClickUrl href Target Option #83
Comments
Yeah, we could do that. Submit a PR 😜 If you do, make sure it also applies to the normal clicks. Looks like here: https://github.com/programcsharp/griddly/blob/master/Griddly/Scripts/griddly.js#L343 -- is your griddly.js out of date? The other option would be to make a rowclick event, handle that, and put in the logic yourself. I think target is fine for now, but if we need to add more logic than that later, we could go to the event and rip out the custom click/modal stuff. I've never been particularly happy with the rowclick we have in general. It's non-standard and js, which means that you can't access via keyboard/screenreader or right click to copy the link. Basic issue is you can't wrap a with an which is what's needed to be semantic. Any ideas to make this better in general? I've seen two approaches:
|
I'll definitely submit a PR. I'll KISS for now and do just the Target piece of this. But I'll keep in mind the rowclick event too. That seems like a good enhancement. |
I just took a look at how the Kendo UI does the select on their grid, and they handle the event, rather than doing anything fancy like a hovering (though that sounded cool). They've got a lot of |
Should we consider this solved? |
A user has requested that the RowClickUrl be enhanced to provide a Target option. I see the code that operates on the e.Which at line 302 of griddly.js.
I would like to add a RowClickUrlTarget attribute that takes a string to allow us to pass in a value. I would put it in a
data-griddly-url-target
value on thetd
and pick it up in the javascript at the same place the url is discovered.Or, is there a way to do this already (beside the ctrl-click option that already exists)?
The text was updated successfully, but these errors were encountered: