Opened 9 years ago

Closed 6 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 9 years ago by Chris Beaven

Needs documentation: unset
Needs tests: unset
Patch needs improvement: set
Summary: Patch for EmailField to render as a mailto anchor in admin interfaceEmailField should render as a mailto anchor in admin list page
Triage Stage: UnreviewedDesign 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 9 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 9 years ago by Chris Beaven

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 9 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 9 years ago by Brian Rosner

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 8 years ago by ssavelan

can I get this working in 1.0.2 ?

comment:7 Changed 6 years ago by Malcolm Tredinnick

Resolution: wontfix
Status: newclosed

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