If a site only supports a 'throbber' for progress, don't show a summary or gear icon

Created on 3 March 2019, about 6 years ago
Updated 8 May 2023, almost 2 years ago

Problem/Motivation

I originally reported this at #2113931-236: File Field design update: Upload field. β†’ .B:

When I went to configure the widgets on "Manage form display", I noticed a minor UI WTF. The summary says "Progress indicator: throbber" but when I click on the gear icon to see what the other choices are and try them out, there's no setting at all. :/ I guess that's supposed to be at πŸ“Œ File Field design update progress bar Active or something, but until that lands, it seems a little cruel and a lot confusing to advertise the "throbber" as a widget option if I can't change it. ;) Can we just rip that out from the settings summary and add it back in once there's a setting we can change that needs to be summarized?

Summary:

Empty widget settings:

I then discovered it has nothing to to with that patch nor πŸ“Œ File Field design update progress bar Active , but instead my local dev site didn't have uploadprogress.so enabled. ;)

Splitting this off as a minor UX follow-up...

Note, this might be made obsolete if we get πŸ“Œ Add support for built-in PHP session upload progress Needs work working and all sites will always have a bar option available and file_progress_implementation() always returns something (or goes away entirely?).

Proposed resolution

If file_progress_implementation() returns FALSE, theres's no progress bar implementation available, and we only support a throbber, don't show a summary for the progress indicator and don't show a gear icon when configuring a file widget.

Remaining tasks

TBD.

User interface changes

Hide the gear icon and settings summary for field field widget on sites that don't allow you to configure the progress indicator at all.

API changes

Probably none.

Data model changes

None.

Release notes snippet

TBD.

πŸ› Bug report
Status

Active

Version

10.1 ✨

Component
File systemΒ  β†’

Last updated about 20 hours ago

Created by

πŸ‡ΊπŸ‡ΈUnited States dww

Live updates comments and jobs are added and updated live.
  • Usability

    Makes Drupal easier to use. Preferred over UX, D7UX, etc.

  • Novice

    It would make a good project for someone who is new to the Drupal contribution process. It's preferred over Newbie.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10
  • First commit to issue fork.
  • Pipeline finished with Failed
    4 months ago
    Total: 472s
    #365223
  • I have made a merge request addressing this issue this as part of Drupalcon Singapore 2024 Contribution Day.
    @larowan came by and suggested to add a message explaining the empty field rather than altering the behaviour of the edit settings gear icon, as this would require EntityDisplayFormBase::buildFieldRow() to traverse all the nested fields inside of widget settings form to check if they are visible.
    Removing the form element from the settings form was also not advisable as this would change the form schema.

  • First commit to issue fork.
  • Pipeline finished with Success
    about 2 months ago
    Total: 349s
    #445192
  • πŸ‡¦πŸ‡ΊAustralia mstrelan

    I read the issue summary and merge request and the proposed resolution doesn't match what's in the MR. I think either approach could be viable but we should pick one and update the issue or MR to match. I also wasn't clear what this issue was for from the issue title, I think we should mention field widgets in the title.

  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia
  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Possible to get a simple test assertion added for this?

  • πŸ‡³πŸ‡ΏNew Zealand atowl

    @smustgraves, little harder to do this as an automated test as we need to mock the extension checker so we can return false in check.
    However, we could refactor it locally and then mock the call to the local function?

    Also, i'm wondering if the proposed solution doesn't quite match the screen shots that were provided now? The proposed solution is to just grey out the options if the extension isn't loaded, but the screenshots simply remove the options. Is that more what we are after? with maybe a message about why?

  • Also, i'm wondering if the proposed solution doesn't quite match the screen shots that were provided now? The proposed solution is to just grey out the options if the extension isn't loaded, but the screenshots simply remove the options

    I can only see one screenshot in this thread, so I am assuming that you are referring to the screenshot in the IS.
    That screenshot is not demonstrating the proposed solution. That screenshot under the "Problem" heading, so it is demonstrating the undesired behaviour.

  • πŸ‡³πŸ‡ΏNew Zealand atowl

    Think the current screen shots are for an older version of drupal, and prehaps outdated.
    I've uploaded some new ones, showing what its currently like in D11, and have left the original screen shot showing what i think the OP wanted it to look like if the extension is loaded.

    The summary of the image should still be shown if the extension isn't loaded.

  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    I agree testing is not feasible here - short of mocking the extension_loaded function in the same namespace as the widget which would be a bit black magic.
    Thanks for updating the screenshots @atowl
    Updating issue credits

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    If no test coverage needed then this is probably good then.

    • longwave β†’ committed 66a2caac on 10.5.x
      Issue #3037204 by starlight-sparkle, atowl, dww, kim.pepper: If upload...
    • longwave β†’ committed 88aafb36 on 11.x
      Issue #3037204 by starlight-sparkle, atowl, dww, kim.pepper: If upload...
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Backported to 10.5.x as a minor UI improvement, but not eligible for 11.1.x or 10.4.x due to the UI change.

    Committed and pushed 88aafb36335 to 11.x and 66a2caacc47 to 10.5.x. Thanks!

Production build 0.71.5 2024