[drupal-devel] [feature] add inline functionaliy to uploads
Bèr Kessels
drupal-devel at drupal.org
Tue Jul 5 12:06:49 UTC 2005
Issue status update for
http://drupal.org/node/26288
Post a follow up:
http://drupal.org/project/comments/add/26288
Project: Drupal
Version: cvs
Component: upload.module
Category: feature requests
Priority: normal
Assigned to: Anonymous
Reported by: Bèr Kessels
Updated by: Bèr Kessels
Status: patch
The function calling the theme function should decide whether its an
image. IMO that is far too hardcore code for a themer ;)
<?php
$image = ereg('^(image/)', $file->filemime);
?>
inside _upload_inline() does the trick.
I did find one issue, though, with svgs, which are image/svg so maybe
we should limit this to really only inline jpg, png, and gif, by
extension? But I am no fan of determining files by extension, and IMO
getimgsize is too heavy;
Bèr Kessels
Previous comments:
------------------------------------------------------------------------
July 3, 2005 - 19:03 : Bèr Kessels
Attachment: http://drupal.org/files/issues/upload_inline.patch (19.05 KB)
One of the most often asked features is proper inlnie handling of files.
Look at the amount of solutions, the popularity of image_assist, and the
amount of peolpe dowloading image.module! That alone should be enough
proof that Drupal lacks proper inline image support.
This patch adds that to core. In fact, it does little more then
appending a link of img tag to the body or the teaser. Off course that
is configurable per file. Next to the [] list checkbox, this patch adds
an [] inline checkbox.
Simplicity is the foundation of this patch. I want no stles for inline
editing, no fancy html wrappers, no tokens, just $node->body or teaser
appended with a small html string.
Another small themable funtion is introduced, (hey, you cannot expect
me to develop something without adding more power for themers, now, can
you? ;) ), that allows people to theme the string that is appended to
the body or the teaser.
Oh, and also note hat the biggest part of this patch is some cleaning I
had to do in order to be able to develop properly. I dont like Ifs
inside cases in foreaches inside swiches. in other words: nodeapi now
calls functions instead of executing code directly.
------------------------------------------------------------------------
July 3, 2005 - 19:19 : Bèr Kessels
Attachment: http://drupal.org/files/issues/inline.patch.screenshot.png (26.68 KB)
here is how the form now looks
------------------------------------------------------------------------
July 3, 2005 - 19:19 : Bèr Kessels
Attachment: http://drupal.org/files/issues/inline.patch.screenshot3.png (30.53 KB)
and this is an example of inlined images and a .doc file.
------------------------------------------------------------------------
July 3, 2005 - 21:44 : sepeck
changing to patch per request from berkes
------------------------------------------------------------------------
July 5, 2005 - 08:46 : Kobus
This gets a +1 from me in principle, however, the [inline:xx] tag with
inline.module gives you greater freedom as to where the inline image
must be displayed. If you can add this functionality (I haven't checked
the code, I don't know if it is in there) it would be a great addition
for Core. This same strategy can be used for inline blocks, I am sure.
Regards,
Kobus
------------------------------------------------------------------------
July 5, 2005 - 09:57 : Bèr Kessels
there are a couple of reasons why i did not include the [inline] tags.
* I aimed for extreme simplicity: a checkbox shows an image inline: its
up to the theme where it appears (if one does not like it before/above
the body and teaser.). Simplicity was the main goal.
* We don't have any tokens in core. And we should not have them.
* Tokens are a very bad substitute for a good interface. They give less
power then plain HTLM. Are much worst documented then HTML, but in the
mean time, they are still as hard to learn as HTML. (Yes, I know people
_think_ they are easier, but there _is_ not difference between [ ] and ,
only that its a different ascii char.
So, no. I don't allow any placement of the image. I leave that that for
dedicated modules, or the themer to decide.
------------------------------------------------------------------------
July 5, 2005 - 10:34 : Kobus
So you will provide an API to do this? For example, with perhaps minor
modifications, the inline.module will be able to display these files?
In that case I am happy. If not, then I can't give my support to this
patch (as if that matters...).
A themer can't do this task as inline images (and blocks for that
matter) is too dynamic for theming; it can be placed anywhere in a node
where the user pleases. This means for me that there should be a module
for this, such as inline module that allows you to define [inline:xx]
tags. If your module emits an array of uploaded files (such as
upload.module), inline.module can be, with minor changes, adapted to
show these files inline.
To show you what I mean with the content is too dynamic for a themer to
perform this task, look at this screenshot related to inline blocks
(inline images can follow the same pattern):
http://drupal.org/files/issues/regions---possibility-3.png
------------------------------------------------------------------------
July 5, 2005 - 12:26 : Bèr Kessels
Kobus,
We are dealing with different issues here: You want a method to place
images, files or so anywhere in the post. That is fine, but certainly
NOT addressed in the patch!
I made patch that *only* adds marked files to the body. its really
nothing more then
<?php
$node->body = $filestring . $node->body
?>
No APIS, no, dynamic tokens, no filters, nothing.
However, what I meant with themers, is that there now is a
theme_upload_inline available, so you can theme the abovementioned
$filestring. On top of that $node->files[FILEID]->inline is TRUE if a
file is flagged for inline.
So in node.tpl.php, or wherever you want to theme a node, you can print
nice images inline, when that flag is set.
And about the comment that a themer cant do this:
Simply not true. On most sites images are always placed in the same
places. REally, even the sites see which use img_assist or inline, use
them to place the images on the exact same places in every node. People
/think/ they want the power to place images anywhere, but they hardly
ever use that power. Just look at all the big news/publishing sites out
there (BBC, CNN, BoigBoing, r even freshmeat) images are all placed acc.
to the theme. They are not placed in random places by authors. So if you
are one of the few that still want that power, there aer mots of power
tools like inline module or img_assist. We should offer a good default,
one that is simple.
------------------------------------------------------------------------
July 5, 2005 - 12:41 : Gerhard Killesreiter
I fully agree that tags are evil.
Kobus: You can always look for tags in $node->body in your theme's node
function.
------------------------------------------------------------------------
July 5, 2005 - 12:50 : stefan nagtegaal
Ber,
the theme-function in your code looks like:
<?php
+/**
+ * Theme function for rendering of inline images
+ * @param $file a file object.
+ * @param $image a Boolean, indicating whether an img tag (TRUE) or an
anchor tag (FALSE) should be used.
+ * @ingroup themable
+ */
+function theme_upload_inline($file, $image) {
+ if ($image) {
+ return '<img src="'. check_url(($file->fid ?
file_create_url($file->filepath) :
url(file_create_filename($file->filename, file_create_path())))) .'"
alt="'. check_plain($file->filename) .'" class="upload inline">';
+ }
+ else {
+ return ''. check_plain($file->filename) .' [1]';
+ }
+
+}
?>
After looking at your code it's not clear to me when $image is TRUE or
FALSE, can you elaborate on me please?
[1] http://drupal.org/'. check_url(($file->fid ?
file_create_url($file->filepath) :
url(file_create_filename($file->filename, file_create_path())))) .'\"
class=\"upload inline
More information about the drupal-devel
mailing list