[drupal-devel] [feature] Allow different content marker types
Goba
drupal-devel at drupal.org
Sat Jan 29 22:16:21 UTC 2005
Project: Drupal
Version: cvs
Component: base system
Category: feature requests
Priority: normal
Assigned to: Goba
Reported by: Goba
Updated by: Goba
Status: patch
Attachment: http://drupal.org/files/issues/Drupal-differentiate-content-markers5.patch (8.14 KB)
OK, Dries is right again :) Updated patch with markers separated. The
form required marker has it's own CSS styling now, and only the content
markers are printed by theme_mark(). Also changed the comment code a bit
to delegate marker display to the theme function. This does add an extra
space after the comment subject if a marker is not printed, so it might
not be desirable. Note though that there has always been these kind of
extra spaces after the node titles in tracker and node modules on the
user interface, and after comment titles on the admin interface. So I
don't think this is becoming a problem.
I hope this patch is going to be fine. :)
Goba
Previous comments:
------------------------------------------------------------------------
January 26, 2005 - 22:47 : Goba
Attachment: http://drupal.org/files/issues/Drupal-allow-different-markers.patch (4.21 KB)
This simple and straightforward patch adds the ability to define
different types of markers (while retaining the old default behaviour
of the new and required markers to look the same). Someone with enough
time on his hands might be able to partition the new marker to a real
new marker and a changed marker (since node_is_new() returns TRUE even
if nodes changed, and not only when they are new). This is the base on
which the new patch can be worked though.
------------------------------------------------------------------------
January 27, 2005 - 07:19 : Dries
Being able to differentiate between 'updated' and 'new' would be a plus.
(Basecamp does it for example, and it has been suggested repeatedly by
Kika/Kristjan.)
------------------------------------------------------------------------
January 27, 2005 - 08:45 : stefan nagtegaal
First of all, i like the patch..
Another usability improvement wou be to have a little helptext at the
bottom of the form which tell you something like:
<?phpreturn t("The fields with a '%marker' are required to fill in.",
array('%marker' => theme('mark', 'required')));?>
Under the tables of tracker, node, comment, and whereever items/users
or whatever may be marked as new, i expect:
<?phpreturn t("The fields with a '%marker' are new.", array('%marker'
=> theme('mark', 'new')));return t("The fields with a '%marker' are
updated.", array('%marker' => theme('mark', 'updated')));?>
What do ou guys think?
------------------------------------------------------------------------
January 27, 2005 - 12:41 : Goba
Stefan, markers are not necesserily inline. At one of my sites, we use a
right floated block element to indicate that something is new. So this
kind of help text will not work... The marker however can contain some
help in itself in a title="" attribute, so that those hovering over it
with their mouse can get an idea on what it is.
------------------------------------------------------------------------
January 27, 2005 - 13:57 : Dries
Committed to HEAD.
------------------------------------------------------------------------
January 27, 2005 - 18:40 : Goba
Attachment: http://drupal.org/files/issues/Drupal-differentiate-content-markers.patch (6.48 KB)
Suprise :) In real incremental patching sense, here is a patch to
introduce the different new and updated makers. It operates with
passing on the info to theme('mark'), and then letting it decide on
what to do with it. This also enables people to do 'read' markers, if
so desired (displaying different icons for read, uread and updated
content).
Renamed the 'new' marker to 'content', so that it is not misleading,
and introduced some constants, to help along the way. 'node' might be a
better name instead of 'content', but it is just a search and replace in
the patch, so Dries can fix it up, if needed. Also introduced the unkown
maker for anonymous users. The practice before was that by setting the
last view time to time(), markers are always displayed as 'read' for
anonymous users. But with the different icons possibility in mind, it
is important to distinguish the case when we cannot decide on whether a
content is new/updated or not, because we have no data. This is why the
unkown marker is introduced.
I admit I have not tested the patch, but it should work well,
theoretically. Kept the original interface behaviour, which does not
expose much from the power of the markers, but preserves the currently
expected output.
------------------------------------------------------------------------
January 27, 2005 - 18:52 : Goba
Attachment: http://drupal.org/files/issues/Drupal-differentiate-content-markers2.patch (7.29 KB)
Wups, forgot to adjust one usage of node_is_new(). Plus renamed it to
node_marker(), since it is not anymore only about checking on whether
it is new or not...
------------------------------------------------------------------------
January 27, 2005 - 19:57 : Anonymous
Shouldn't 'MARKER_UNKOWN' be 'MARKER_UNKNOWN'? Why do we need an
"unknown" possibility anyway? Wouldn't we just omit markers for
anonymous users?
------------------------------------------------------------------------
January 27, 2005 - 22:55 : Goba
Markers are only important for logged in users, yes. We need to somehow
omit the markers. We can check for the user in theme_mark(), but we
need to check for the user in node_marker() anyway, since otherwise we
would do an unnecessery database lookup for each marker. So we can
return MARKER_READ in node_marker() and omit the marker regardless of
the $marker if the $type is 'new' and the user is not logged in. So we
need to check for the user in the theme function too...
BTW the terminology I introduced seems to be inconsitent: node_marker()
vs. theme_mark(). It might be desireable to rename the theme function,
or use MARK_ constants and node_mark(). Pick the way.
------------------------------------------------------------------------
January 29, 2005 - 18:37 : Goba
Attachment: http://drupal.org/files/issues/Drupal-differentiate-content-markers3.patch (7.42 KB)
Since noone hinted on a way to pick, I needed to decide myself :) Gone
is the unkown marker, and now it is the theme's task to not display
content markers for anonymous users (while still keeping the required
marker for them). The contants and functions were renamed to use 'mark'
instead of 'marker' in line with the existing theme_mark() function.
Still the same functionality, but with less code compared with the
previous patch.
------------------------------------------------------------------------
January 29, 2005 - 21:52 : andremolnar
I think it might be helpful if you expanded the phpdocs for the constant
definitions to describe when, where, how they are used. Right now the
text only says 'Markers used to designate content.'
------------------------------------------------------------------------
January 29, 2005 - 22:02 : Goba
Attachment: http://drupal.org/files/issues/Drupal-differentiate-content-markers4.patch (7.53 KB)
Updated patch, which links to theme_mark() and node_mark(). Otherwise I
think the names of the constants really tell everything.
------------------------------------------------------------------------
January 29, 2005 - 22:14 : tangent
I find the new "content" state to be less meaningful than the old "new"
state. The word "new" is an adjective while "content" is a noun which,
to me, applies to everything.
Perhaps "unread" would be a more appropriate state since that is the
content that the marker identifies.
------------------------------------------------------------------------
January 29, 2005 - 22:39 : Goba
Updated is not unread. Also there could be 'read' marks. Your email
client has marks for read and unread messages doesn't it? Drupal will
allow this too. If we rename 'content' to 'unread', then the unread
marker would be able to signify that something is changed (while it was
already read) and that it was read (which is odd for an unread mark you
must admit :).
------------------------------------------------------------------------
January 29, 2005 - 22:55 : Dries
Removing the unknown-marker makes sense to me. Personally, I'd split
the function theme_mark() because 'required' and 'content'
(read/new/updated) are really two different things now. I'd hard-code
the required part in theme_form_element() using proper CSS-classes.
IMO, that would simplify the code and make it a tad more readable.
(Initially, I couldn't figure out what the 'content' parameter was
for.)
--
View: http://drupal.org/node/16253
Edit: http://drupal.org/project/comments/add/16253
More information about the drupal-devel
mailing list