backdrop-contrib / socialfield

The Social field module provides a field that allows you to collect links to social network profiles like facebook, twitter, google+, linkedin, etc.
GNU General Public License v2.0
2 stars 4 forks source link

Use of HTML ID that could cause issues #8

Closed wesruv closed 1 month ago

wesruv commented 8 years ago

https://github.com/backdrop-contrib/socialfield/blob/1.x-1.x/socialfield.theme.inc#L51

There could be a page that has two of these, is there a reason this couldn't be a class instead?

CybersecDan commented 7 years ago

I realize this is from 10 months ago, so things may have changed since, but I wanted to share a quick thought. I'm in favor of using a class instead. I tried to create an instance where the id socialfield-table would render more than once on a page (forgive me if you're already familiar or have done this already). Seems like it would only be rendered as a table in the admin theme. I didn't see any duplication of the id on the public side.

I created two fields in a content type and use social field for both. When I was creating the content I noticed a little bug where I couldn't remove services from the second socialfield field. However, it did allow me to fill in a handle w/url and publish it. Then going back in to edit the page I only saw the one handle w/url in each field. (Err.. hope this makes sense.)

Just thought I'd throw that out there, for what it's worth. I didn't do much more digging after that (Getting late here on the east coast). :)

jenlampton commented 1 month ago

Unfortunately backdrop_add_tabledrag() still requires an ID, but I have made them unique for each table. This fixes tabledrag, but the "Add more" option still doesn't work for multiple socialfields on the same form...

jenlampton commented 1 month ago

I'm going to create a separate issue for that. It's related, but it is going to require a more comprehensive fix. Closing.