Opened 6 months ago

Closed 6 months ago

#22613 closed enhancement (fixed)

Media modal: Show a visual indication when settings will be saved

Reported by: ocean90 Owned by: markjaquith
Priority: low Milestone: 3.5
Component: Media Version: 3.5
Severity: normal Keywords: has-patch commit
Cc:

Description

Since it's not clear, that the new media modal saves the settings on the fly, we should add a visual indication whenever the settings are saved.

Patch is a beginning, feel free to continue.

Attachments (8)

22613.patch (2.6 KB) - added by ocean90 6 months ago.
22613-option-b.diff (3.2 KB) - added by lessbloat 6 months ago.
22613.refresh.patch (2.6 KB) - added by lessbloat 6 months ago.
22613-option-b.refresh.diff​ (3.3 KB) - added by lessbloat 6 months ago.
22613.2.diff (3.8 KB) - added by koopersmith 6 months ago.
22613.diff (1.6 KB) - added by nacin 6 months ago.
22613.3.diff (4.0 KB) - added by helenyhou 6 months ago.
22613.4.diff (4.8 KB) - added by nacin 6 months ago.

Download all attachments as: .zip

Change History (25)

Good start.

I wanted to see if I could get the spinner right next to each input in 22613-option-b.diff​. Not saying it's better - just another option.

  • Keywords needs-refresh added

To apply cleanly to trunk, both patches need the first chunk of media-views.js refreshed.

refreshing: 22613.refresh.patch, 22613-option-b.refresh.diff​​

  • Keywords needs-refresh removed

I prefer 22613-option-b.refresh.diff%E2%80%8B, despite something that happened to the filename that makes Trac not highlight it :) Took me a while to find the spinner placed at the top - would probably not have noticed it if I wasn't looking.

In both cases the spinner does not go away if you've emptied the field, though it does seem to save the data successfully.

  • Keywords has-patch needs-testing added; needs-patch removed

Patch implements the single-spinner variant, accounting for re-renders, multiple requests, and race conditions.

Works! Want to make sure it doesn't collide with the "Attachment Details" text in translations, though - does funky things if they don't fit next to each other.

In 22991:

Show a spinner when attachment details are saved.

Props koopersmith
see #22613

  • Resolution set to fixed
  • Status changed from new to closed

I inserted a huge string for Attachment Details and it seemed to handle it pretty well.

Shouldn't we do this for the gallery view too? For the description field?

Probably totally unreasonable, but I managed to do this:

http://f.cl.ly/items/0A0N1T3G2u000z1E0B47/Screen%20Shot%202012-12-03%20at%207.31.58%20PM.png

Fine with calling that a non-situation, though.

  • Keywords needs-patch added; has-patch needs-testing removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

With the updated UI I think we should change the place of the spinner since it can be hidden now.

We should place it in the top bar (near the X), so it can be shown for the gallery view too.

  • Priority changed from normal to low

nacin6 months ago

nacin6 months ago

  • Keywords has-patch added; needs-patch removed

Testing 22613.4.diff, logging in (or rather, switching to) the following users. I have categories enabled for attachments, which shows a compat field, and uploaded attachments as administrator and author users.

  • Author (the important part)
    • Before: Can edit any field in all views but it won't actually save, although input will remain in the field for the duration of the page load for all views (not sure if this is desirable). User input does insert with the item.
    • After: When in the insert workflow, it is the same, except that category input is readonly for items not uploaded by the user. When in the gallery workflow, all fields are readonly except for those for items uploaded by the user, including inline caption field.
  • Administrator and Editor
    • Before: can edit any field and it will save
    • After: same
  • Contributor
    • Before: no access to the modal
    • After: same
  • Keywords needs-testing added
  • Keywords commit added; needs-testing removed

Looks good. +1.

  • Owner set to markjaquith
  • Resolution set to fixed
  • Status changed from reopened to closed

In 23072:

Intelligently make media fields readonly when the user cannot update or do anything with them. props koopersmith, helenyhou, nacin. fixes #22613

Note: See TracTickets for help on using tickets.