Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
See attached images:
Current:
Proposed:
Comment | File | Size | Author |
---|---|---|---|
#14 | drupal-filefield-text-2041629-14.patch | 3.8 KB | superspring |
#6 | shorten-text-under-image-upload-2041629-6.patch | 2.55 KB | manningpete |
#11 | drupal-filefield-text-2041629-11.patch | 3.78 KB | mcrittenden |
#10 | extra_dot.jpg | 15.97 KB | CaptainWonky |
#9 | drupal-filefield-text-2041629-9.patch | 3.79 KB | mcrittenden |
Comments
Comment #0.0
tkoleary CreditAttribution: tkoleary commentedsized image
Comment #1
larowlanTagging
Comment #2
ptkoroma CreditAttribution: ptkoroma commentedI do agree with this approach
Comment #3
reglogge CreditAttribution: reglogge commentedA couple of notes on that one:
- The first line "Upload an image to go with this article" comes from the "standard" install profile and therefore can be easily overridden by either a custom install profile or - even easier - in the configuration screen for each such field.
- The following lines of text can also simply be overridden by a site developer since they come from a theme() function. The relevant code for the function theme_file_upload_help() lives in core/modules/file/file.field.inc.
- The allowed file size, file types and number of files that can be uploaded are not fixed but can be changed through simple configuration. The llst of allowed file types for example could get quite long.
- There are additional configuration options for image fields relating to allowed pixel dimensions of images to be uploaded. These would also display when activated by a site developer.
Pulling all these in part dynamic strings together into one line could easily result in something rather long and ugly. I personally would keep everything as it is and leave it up to site developers to override this display if the want to on an individual basis.
Comment #4
tkoleary CreditAttribution: tkoleary commented@reglogge
I think our basic assumptions about how Drupal 8 should appear to a new user are not matching. From you post it seems that you look at a new install as a kind of template or raw material for what will become a CMS once a developer has spent some time configuring it.
The approach we started to take in Drupal 7 and continued in 8 is to provide the user with a CMS that just works out-of-the-box. With that in mind my suggestion would by the inverse approach to yours: give the user the most minimal and least obtrusive description and allow the developer to add to it if they find it necessary.
On your last comment about having things on one line, this could be remedied by setting each string to display inline-block so that they would wrap more naturally based on the content.
Comment #5
reglogge CreditAttribution: reglogge commented@tkoleary: Right you are. This is indeed a recap of the age old question whether Drupal is a framework or an application. I apparently am more on the framework side of things but can totally see your point.
Changing the texts is technically trivial (that's why you tagged the issue with "Novice") so the only question is: should we? I wouldn't, but then again, who am i to decide this :-)
Anyway, setting this to "Needs work" since there is no patch yet.
Comment #6
manningpete CreditAttribution: manningpete commented1. I emptied the default description "Upload an image to go with this article." from field.instance.node.article.field_image.yml (devs can still add description to that field later if/when they want)
2. I changed the order of the statements, so cardinality would appear first per the proposed example.
3. I removed the strong tags per the proposed example.
4. I simplified the language per the example.
5. I removed the br tag from the implode, and added a space (without the break tag, everything appears on the same line and no inline-block was required)
Comment #8
tkoleary CreditAttribution: tkoleary commentedHow about:
One file, 256 MB max, of type: png gif jpg or jpeg.
I know inserting the "or" before the last child in the types string is a challenge but it would be a nice touch. :)
Comment #9
mcrittenden CreditAttribution: mcrittenden commentedAttempting to fix the tests.
Comment #10
CaptainWonky CreditAttribution: CaptainWonky commentedManual testing on latest patch shows an extra dot after the size limit:
Comment #11
mcrittenden CreditAttribution: mcrittenden commentedWhoops, that'd be my fault. Fixed here.
Comment #12
tkoleary CreditAttribution: tkoleary commented@mcrittenden
Looks terrific. This is a big improvement.
Comment #13
Wim Leers'limit' isn't part of the
t()
call, so it's not translatable.Using index 0 here and above, shouldn't that be the index of the current item in the field?
Comment #14
superspring CreditAttribution: superspring commentedI've updated mcrittenden's patch to reflect the review in #13.
1. Limit is now translatable
2. According to the API, the 0th's index does not refer to field's index - see https://api.drupal.org/api/drupal/includes%21file.inc/function/file_vali... for more details
Comment #15
tkoleary CreditAttribution: tkoleary commented@Wim Leers, @superspring
I would like to mark this patch RTBC but I think Wim should validate whether it addresses his comments.
Comment #16
Wim LeersNo interdiff was provided, so I'll assume that "limit" thing was the only change. In which case, RTBC.
Comment #17
webchickYay, less verbosity! :) This is quite a bit longer patch than one might think at a glance.
Committed and pushed to 8.x. Thanks!
Comment #18
tkoleary CreditAttribution: tkoleary commented@superspring, @mcrittenden, @manningpete, @larowlan, @Wim Leers, @webchick, @CaptainWonky, @reglogge, @ptkoroma, Thank you all. This is a massive improvement.
Some may see this type of issue as a small thing but this is exactly the type of attention to detail in the authoring experience that adds up to a "delightful" experience for the user.
While we're at it here's another small change with big impact:#2018871: Make summary and details collapsed by default
:)
Comment #18.0
tkoleary CreditAttribution: tkoleary commentedagain