Opened 8 years ago

Closed 5 years ago

#5767 closed (wontfix)

EmailField should render as a mailto anchor in admin list page

Reported by: hax Owned by: nobody
Component: contrib.admin Version: newforms-admin
Severity: Keywords: nfa-someday EmailField
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

It seems to me that there is no reason to not have an EmailField be a mailto link in an admin interface. These interfaces generally aren't public, so hiding from spambots shouldn't be an issue.

This is my first attempt at a patch, feedback is appreciated :)

Index: django/contrib/admin/templatetags/admin_list.py
===================================================================
--- django/contrib/admin/templatetags/admin_list.py     (revision 6525)
+++ django/contrib/admin/templatetags/admin_list.py     (working copy)
@@ -177,6 +177,12 @@
                     result_repr = ('%%.%sf' % f.decimal_places) % field_val
                 else:
                     result_repr = EMPTY_CHANGELIST_VALUE
+            # EmailFields are mailto links
+            elif isinstance(f, models.EmailField):
+                if field_val is not None:
+                    result_repr = '<a href="mailto:%s">%s</a>' % (escape(field_val), escape(field_val))
+                else:
+                    result_repr = EMPTY_CHANGELIST_VALUE
             # Fields with choices are special: Use the representation
             # of the choice.
             elif f.choices:

--maqr

Change History (7)

comment:1 Changed 8 years ago by SmileyChris

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Summary changed from Patch for EmailField to render as a mailto anchor in admin interface to EmailField should render as a mailto anchor in admin list page
  • Triage Stage changed from Unreviewed to Design decision needed

Hi hax, the normal way is to attach the patch as a file rather than paste it in your description.

Since admin is being replaced, you'd be better off creating a patch against the newforms-admin branch.

comment:2 Changed 8 years ago by hax

That is against the newforms-admin branch, I'm not sure why it would look like it's not.
In the future I'll attach the .diff, but from my svn info:
URL: http://code.djangoproject.com/svn/django/branches/newforms-admin
Repository UUID: bcc190cf-cafb-0310-a4f2-bffc1f526a37
Revision: 6525

So I'm pretty sure I patched for the right branch.

Let me know,

--hax

comment:3 Changed 8 years ago by SmileyChris

Oh, silly me - how did I overlook that? For penance, I'll properly review the patch :)

  1. I don't see any benefit of falling back to EMPTY_CHANGELIST_VALUE for a text field
  1. Since your if only checks for is not None, an empty string would return '<a href="mailto:"></a>'

comment:4 Changed 8 years ago by hax

Oops :)

I'm pretty sure this is right (and for some reason, it won't let me attach in a reply, or I'd upload it as a diff):

Index: django/contrib/admin/templatetags/admin_list.py
===================================================================
--- django/contrib/admin/templatetags/admin_list.py     (revision 6525)
+++ django/contrib/admin/templatetags/admin_list.py     (working copy)
@@ -177,6 +177,12 @@
                     result_repr = ('%%.%sf' % f.decimal_places) % field_val
                 else:
                     result_repr = EMPTY_CHANGELIST_VALUE
+            # EmailFields are mailto links
+            elif isinstance(f, models.EmailField):
+                if field_val:
+                    result_repr = '<a href="mailto:%s">%s</a>' % (escape(field_val), escape(field_val))
+                else:
+                    result_repr = ""
             # Fields with choices are special: Use the representation
             # of the choice.
             elif f.choices:

--hax

comment:5 Changed 7 years ago by brosner

  • Keywords nfa-someday added

This ticket isn't critical to merge newforms-admin into trunk. Tagging with nfa-someday. While I see this as a good feature and no reason not to include it, it can already be accomplished. list_display can accept callables so you can define a method on your model and get the same functionality.

comment:6 Changed 6 years ago by ssavelan

can I get this working in 1.0.2 ?

comment:7 Changed 5 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from new to closed

We don't do this kind of thing for URL fields, so it would be inconsistent. We also have one column int he changelist page that is a link that goes to add form and that would cause confusion about what the links do.

Creating a clickable column in the admin list page is easy enough: write a function and use that in the "fields" attribute (enabling linking for that function, etc), so creating this effect can be done in admin classes without requiring Django source changes.

Note: See TracTickets for help on using tickets.
Back to Top