8000 Added the ability to have table rows with object ids by philippfrank · Pull Request #1 · mbreit/tablebuilder · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Added the ability to have table rows with object ids #1

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Added the ability to have table rows with object ids #1

wants to merge 3 commits into from

Conversation

philippfrank
Copy link

No description provided.

@mbreit
Copy link
Owner
mbreit commented Jul 5, 2013

First of all, thank you very much for your pull request. It is great to see someone else interested in this gem.

There are two thoughts that I would like to discuss before merging it, though:

  • We should use the helper methods from ActionController::RecordIdentifier that create classes and ids for HTML elements that are consistent with other helper methods like content_tag_for.
  • I think this should be configurable for two reasons:
    1. Adding the ids of the object to the HTML code might leak sensitive informations under some circumstances.
    2. Element ids have to be unique within the whole page and this could conflict with other uses of content_tag_for.

I am not sure about how to call this options and whether this should be on by default.

@philippfrank
Copy link
Author

I like the gem since it is so clean and simple. Nevertheless it could get some more documentation and features. In this case you're right. I added this as an option so one has to explicitly turn it on. Also i use dom_id for the id generation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@philippfrank @mbreit
0