Allow users to inline update the variant of an image in admin #1580

mtomov · 2016-11-08T20:07:59Z

This should allow users to update the variant of an image directly within the admin images table.

I have pretty much copy/pasted the code from the payments/edit class, so thanks @jhawthorn for that! Indeed a great learner on syncing Backbone's models with the backend.

Not sure about the dotted underline below the small icons .. maybe I can remove that, but couldn't find an appropriate class for the purpose.

tvdeyen · 2016-11-08T22:24:11Z

Great feature. Didn't had the time to loon into the implementation details, but this looks good on a first glance.

One thing coming to mind, though. From a UX point of view I'm asking myself why we don't always show the select box With the current one preselected. This spares some clicks and we could get rid of the extra icons.

Thoughts @Mandily ?

jhawthorn · 2016-11-14T23:42:03Z

I'm a little concerned that having two edit buttons will be confusing to users. @tvdeyen's suggestion would solve this.

Mandily · 2016-11-15T00:58:08Z

I have a couple of thoughts on preselecting values in dropdowns.

It's great if most of the time the user would select the same option anyway. This saves time, and cognitive load.
It's not so great when you want a user to actually think about what's in a box. You can risk them skimming over it and setting something up improperly.

Without doing a full in depth exercise, I suspect that there's a good deal of dropdowns that would be suitable for a default value.

With regards to the edit functionality of the tables - I think this is a great idea and I secretly thought it was going to take a lot longer before we'd start to see more of this added into the admin. So I'm excited you're doing it now @mtomov.

I agree with Thomas and John - Instead of using additional icons within the tables, it would be nice to use the same convention that the product stock screen uses - swapping out the edit and delete buttons for a checkmark and X. I think it streamlines both the interaction and the UI.

In the future, we could take this one step further and make everything in the page editable without having to go into an edit mode, and automatically save changes on field blur. But in the meantime I'm more than happy to see edit functionality being merged into the view pages themselves.

mtomov · 2016-11-15T21:53:32Z

Thanks @Mandily and the others for the feedback. It'll be great indeed if we have a standard for those.

I just didn't fully understand what the final look would be with the checkmark and X buttons - do they go next to the current Edit and Delete buttons, is the select box always there available for change (no need to press Edit as Thomas suggested)?

The only drawback of having the selectbox immediately there available is that it might look a bit ugly, as the deep blue on those select boxes puts a great focus on the area, as if though something is off ..

Naturally, it's actually easier indeed from a programming point to have the select box directly there available .. so it's really up to you on how you're best seeing this.. as either option is sort of the same programming effort if you are concerned on that ..

Mandily · 2016-11-15T23:21:53Z

The checkmark and X buttons replace the edit and delete buttons when the user is in edit mode. Clicking on edit makes the whole row editable. Once the checkmark or X is clicked, the row goes back to view only, the edits are saved, and the edit and delete buttons appear again.

I don't think we should avoid introducing a new and improved interaction model because a visual style is too garish. At some point I'd either like to update the select 2 inputs to v4 or restyle them ourselves with the rebrand. @jhawthorn might be better able to speak to that though.

I realize this would also include a visual style change in the tables, but the best way I've seen this editable table thing done is that when a user hovers over a table row, it gets highlighted with the fields marked in white.

I believe this is the best model, but before we go that way, I would want to understand if it's something that you think is doable across Solidus as a whole in a relatively short time period, because I would hate for a transition like this to take 4 years to complete.

mtomov · 2016-12-01T23:22:52Z

Amanda, the hover suggestion was brilliant, did it, wasn't difficult at all actually, hopefully can also be used by other similar in-table editing functionalities.

With the hover, it's quite incredible how quickly one could change the variant on a whole bunch of images.

9e2eb1a was the actual commit with the change

on hover

on click

.. finally the user needs to click on the check icon to confirm the change

mtomov · 2016-12-05T15:50:33Z

I've actually gone about to re-write the javascript code for inline image update into a generic reusable Editable Table "plugin" of sorts, so that tables can be turned into inline editable only with html markup, similarly to the sortable plugin and such.

It's all in the last commit, and as an example, to do it on a text field element in cell, one would need to create such markup:

  <td>
    <span class="editing-hide">
      <%= image.alt %>
    </span>

    <%= fields_for image do |f| %>
      <%= f.text_field :alt, class: 'editing-show' %>
    <% end %>
  </td>

Now, to eliminate a bit of duplication here - an improvement could be to check if the cell has an element with editing-hide, and if not - then create it using the value of the input (select) field. That would reduce the html needed for such table to just an input box with a class of editing-show

Another good improvement is to have the Cancel button revert the input box value it its original one. Should be easily doable though this one.

In this particular case for the image - I don't think we need to have the alt box be inline-editable, but have changed it for demonstration purposes. Might have to add a test if we are to keep it.

--

I think the option types/values admin section could be a good candidate for this kind of UI

Looking forward to any input if that is in the right direction. Thank you!

Mandily · 2016-12-05T19:09:00Z

I downloaded your code and played around with the new functionality. I like it a lot!

It would be nice if the styles between view and edit mode were the same, but I'm not sure if that's something we should tackle with this or leave it for the future rebrand. Especially as we can't do much with the dropdowns right now. What's your take on that? This might even be something I can do myself if you didn't want to get into the finicky css stuff.

I know we technically don't need a save button on this page because we're auto saving, but I think we need to keep the save button on every page for peace of mind for the user. In the future, I'd like to introduce states to buttons, so if the page was saved, the button would look disabled, whereas if it needed to be saved, it would be brighter to attract attention. We don't need to do that for this feature, but I'd like to keep the save button here for future use.

The third thing I wanted to bring attention to was that we don't have a stop gate for catching the user while in the middle of edit mode. If they start to edit a row and don't click on the check or x, and then navigate to another page, changes are lost. This is a behaviour that is consistent across Solidus, but I'd like to do something to remedy it in the near future: I see three options for this:

Auto save rows when a user clicks outside of the row. This would negate the need for check and X buttons
notify the user when they try to leave the page with a modal window.
both of the above

I'm a little more on the side of the adding this kind of functionality here than the save button, but I could be persuaded to add it in later if you had a good reason to leave it.

WRT your most recent post, I like the idea of keeping the alt text an editable field. I was actually trying to figure out how we could do edit the thumbnail image too, but I think that gets more complicated than we want to get at this point. The good thing about making every field in a table editable is that we can then get rid of the edit pages. (WOOHOO!) I think the option types page you mentioned and a good handful of others would benefit from users being allowed to edit right in the table and getting rid of the edit pages altogether. In some cases we might have to add a few more columns to the table, but it shouldn't be too big of a deal.

Let me know if you want to hash out any more ideas or need help identifying those pages. I've got an ongoing list that I can add those "optional" features I mentioned above, so let me know you're thoughts on the stop-gate solutions so I can log them in there if you decide to do them later.

Thanks so much for taking this on!

tvdeyen · 2016-12-05T20:04:53Z

The good thing about making every field in a table editable is that we can then get rid of the edit pages. (WOOHOO!)

I don't think this a working for all tables. Often tables are just a list with little detail and the forms have way more options than ever will fit into a table row. Think of the product or order tables. For small tables I like the idea, though. Although I don't like auto save. What if the user change its mind or mistyped something?

I like the current state with save and cancel buttons, although I think they should look different from the edit and delete buttons. It's very hard to tell for the user if the buttons currently visible are the edit buttons or save buttons. Maybe we should hide all buttons but the ones from the hovered row? Or we highlight the buttons if we are currently editing the row. I kinda like the first option. Would remove clutter from the views.

Mandily · 2016-12-07T18:50:14Z

I agree, I don't think this would be appropriate for all tables, but any of the smaller ones where all data can be represented in a single row should eventually be moved over to something like this. To clarify - it's EVERY FIELD in a small table, not EVERY TABLE.

Contrary to your auto-save point - if the user mistyped something, they can just go back in and edit it again. It's not like we're asking them to confirm with a dialog when they save it anyway, so that extra step isn't really reducing the risk of human error.

I was also thinking on the styles that the check and X are presented in vs the edit buttons. If we remove the other row's buttons, we do remove the clutter, but we also remove the user's ability to edit more than one row at a time. At first I was thinking that would be a good idea, but the more I think on it, the more I think we need to give users the power and freedom to edit these things as quickly and easily as possible. I can't say for sure, but I imagine that when a user needs to edit something on one of these pages, it's probably more than just one row. This is also why I like the auto-save feature. It allows users to be more efficient in entering/changing their data.

In short - Solidus users are regular users of this software, I don't think we need to protect them from themselves as much as we would with a software that many people use only a few times.

mtomov · 2016-12-08T23:31:10Z

Thanks for your comments!

Another round of updates in 0ac12bc focusing on simplification - have removed all the custom classes, reducing everything to a single inline-editable that need be applied to the table. Have also removed the double specification in the cell - we are left with a single input/select box in the cell.

On suggestion from Thomas - have removed all buttons on initial view. On hover - the Edit and Delete buttons come up. On Edit - the Update and Cancel buttons come up.

I've dug out nice row context colours from spree's code depending on the button that you hover over. Have attached some screens as you might've only seen the Destroy (red) context colour and not the others. Hopefully that would help with the confusion between the buttons that you mentioned. Indeed would be nice if we have some colour in those buttons by default.

Finally, have added a real cancel functionality that I mentioned before would be nice to work.

Context colours:

Mandily · 2016-12-09T22:37:08Z

By any chance can we get the cancel tooltip to be orange to match the rest? The red is associated with delete/destroy in my mind.

mtomov · 2016-12-28T11:16:07Z

Sure, updated to orange in the last commit

Rob117 · 2017-02-13T13:52:06Z

Is this not getting merged? If so, can we close it?
It looks useful, but leaving it open in the branches without comment for a month doesn't seem to be doing it justice.

mtomov · 2017-03-23T23:26:53Z

I've rebased it onto latest master. Let me know what else I can do to demonstrate the achieved improvement.

mtomov · 2017-05-22T21:52:57Z

Hello,

Although not a brand new PR, in the latest commit after speaking with @Mandily and @tvdeyen, I've:

Updated the styles to match the new Select2 styles
Added recognition of Enter and Esc keys when inline-editing a table row, as well as tests for both events
Have put back the Edit and Delete buttons to always be visible to keep the interface the same as existing tables
Re-organized the code to match the new Backbone Views organization

Thank you for the feedback!

mamhoff

I like the feature, and have glanced over the code, but would like a second pair of eyes.

Mandily · 2017-05-25T12:34:52Z

This looks great! I love how a small code change can make such a big difference to the usability of the admin.

@mtomov Eventually I'd like to get to the point where we remove the buttons on the side and only show them on hover. Do you have any ideas/timeline on when we should implement that part? I'm also curious to know what the plan is for applying this code to the rest of the tables in the Solidus admin.

mtomov · 2017-05-26T07:50:26Z

Hi @Mandily - happy that you find it usable!

Remove the buttons and show them on hover - technically, doing that requires 1 line of css change. As I've noted above, I did have the right hand side buttons showing only on hover, and to me it looked good, but I've switched them back to always show as this was your preference.
Ideas / timeline - that's all up to you
Plan for applying to other tables - I think someone must figure out what other tables could use a similar functionality, as they aren't that many suitable I think. Then, create an issue to describe the task, which someone might pick and make the code changes.

Thanks @mamhoff for reviewing!

mtomov · 2017-06-13T22:14:36Z

Come on, one more approve : )

Let's see if the new members would be more easily wooed @kennyadsl

Mandily · 2017-06-13T23:50:28Z

I will make a new issue in the next couple of days with other tables and link back to this one.

tvdeyen

I have some comments (sorry, this review was pending and I forgot about it)

tvdeyen · 2017-05-23T13:53:04Z

backend/app/assets/javascripts/spree/backend/views/editable-table.js.coffee

+ when @ESC_KEY then @onCancel(e)
+
+
+class Spree.Views.EditableTable


Can we split this up into separate files?

tvdeyen · 2017-05-23T13:54:30Z

backend/app/assets/stylesheets/spree/backend/shared/_tables.scss

+
+
+/* Inline Editable tables */
+.inline-editable tr:not(:hover):not(.editing) {


I think we should rename the class into inline-editable-table when we have this in the global namespace like this.

tvdeyen · 2017-05-23T13:56:23Z

backend/app/helpers/spree/admin/navigation_helper.rb

@@ -102,7 +102,7 @@ def link_to_delete(resource, options = {})
 url = options[:url] || object_url(resource)
 name = options[:name] || Spree.t('actions.delete')
 confirm = options[:confirm] || Spree.t(:are_you_sure)
- options[:class] = "delete-resource"
+ options[:class] = (options[:class].to_s + " delete-resource").strip


Several Ruby style guides recommends:

"#{options[:class]} delete-resource".strip

@Mandily

- by suggestion from @Mandily as the red is The red is associated with delete/destroy

mtomov · 2017-06-15T12:19:48Z

Brilliant, thanks a lot @tvdeyen - all great spots!

I think I've now addressed them all, and have squashed all my work into a single commit, which should describe it all.

- On clicking an image row, users are able to select the variant of the image from a dropdown and persist the change using Enter from keyboard or the OK button to the right of the row - Use Escape or Cancel button to revert back without persisting the change The editing table functionality is generic enough to be used accross on similar table to allow users to edit the contents of the tables inline. Instructions to turn a table into an inline-editable one: 1. Add `inline-editable-table` class to the table 2. Add `check` and `cancel` buttons, and specify the update url on the `check/save` button. 3. Change the static text into input fields See `backend/app/views/spree/admin/images/_image_row.html.erb` for a usage example

tvdeyen · 2017-06-26T07:56:58Z

🎊 Thanks!

I made a followup #2042

mtomov · 2017-06-26T11:48:07Z

Thanks all for helping out!

jhawthorn added changelog:solidus_backend Changes to the solidus_backend gem type:enhancement Proposed or newly added feature labels Nov 22, 2016

mtomov mentioned this pull request Apr 21, 2017

Reorganize Image upload JS #1848

Merged

mamhoff added the SolidusConf Hack Day label May 23, 2017

mamhoff approved these changes May 23, 2017

View reviewed changes

tvdeyen requested changes Jun 14, 2017

View reviewed changes

Change cancel button tooltip to orange from red

764f185

- by suggestion from @Mandily as the red is The red is associated with delete/destroy

tvdeyen approved these changes Jun 15, 2017

View reviewed changes

tvdeyen mentioned this pull request Jun 26, 2017

Localize each store solidusio/solidus_i18n#95

Merged

tvdeyen merged commit 9e00618 into solidusio:master Jun 26, 2017

tvdeyen mentioned this pull request Jun 26, 2017

Add more inline editable tables #2042

Closed

2 tasks

mtomov deleted the ajax-update-image-variant branch June 26, 2017 11:48

SamuelMartini mentioned this pull request Feb 7, 2020

Scope image by product_id or variant_id in ImagesController #3510

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow users to inline update the variant of an image in admin #1580

Allow users to inline update the variant of an image in admin #1580

mtomov commented Nov 8, 2016

tvdeyen commented Nov 8, 2016

jhawthorn commented Nov 14, 2016

Mandily commented Nov 15, 2016 •

edited

Loading

mtomov commented Nov 15, 2016 •

edited

Loading

Mandily commented Nov 15, 2016

mtomov commented Dec 1, 2016 •

edited

Loading

mtomov commented Dec 5, 2016 •

edited

Loading

Mandily commented Dec 5, 2016

tvdeyen commented Dec 5, 2016

Mandily commented Dec 7, 2016

mtomov commented Dec 8, 2016 •

edited

Loading

Mandily commented Dec 9, 2016

mtomov commented Dec 28, 2016

Rob117 commented Feb 13, 2017

mtomov commented Mar 23, 2017

mtomov commented May 22, 2017

mamhoff left a comment

Mandily commented May 25, 2017

mtomov commented May 26, 2017

mtomov commented Jun 13, 2017

Mandily commented Jun 13, 2017

tvdeyen left a comment

tvdeyen May 23, 2017

tvdeyen May 23, 2017

tvdeyen May 23, 2017

mtomov commented Jun 15, 2017

tvdeyen commented Jun 26, 2017

mtomov commented Jun 26, 2017

		when @ESC_KEY then @onCancel(e)


		class Spree.Views.EditableTable



		/* Inline Editable tables */
		.inline-editable tr:not(:hover):not(.editing) {

Allow users to inline update the variant of an image in admin #1580

Allow users to inline update the variant of an image in admin #1580

Conversation

mtomov commented Nov 8, 2016

tvdeyen commented Nov 8, 2016

jhawthorn commented Nov 14, 2016

Mandily commented Nov 15, 2016 • edited Loading

mtomov commented Nov 15, 2016 • edited Loading

Mandily commented Nov 15, 2016

mtomov commented Dec 1, 2016 • edited Loading

on hover

on click

mtomov commented Dec 5, 2016 • edited Loading

Mandily commented Dec 5, 2016

tvdeyen commented Dec 5, 2016

Mandily commented Dec 7, 2016

mtomov commented Dec 8, 2016 • edited Loading

Mandily commented Dec 9, 2016

mtomov commented Dec 28, 2016

Rob117 commented Feb 13, 2017

mtomov commented Mar 23, 2017

mtomov commented May 22, 2017

mamhoff left a comment

Choose a reason for hiding this comment

Mandily commented May 25, 2017

mtomov commented May 26, 2017

mtomov commented Jun 13, 2017

Mandily commented Jun 13, 2017

tvdeyen left a comment

Choose a reason for hiding this comment

tvdeyen May 23, 2017

Choose a reason for hiding this comment

tvdeyen May 23, 2017

Choose a reason for hiding this comment

tvdeyen May 23, 2017

Choose a reason for hiding this comment

mtomov commented Jun 15, 2017

tvdeyen commented Jun 26, 2017

mtomov commented Jun 26, 2017

Mandily commented Nov 15, 2016 •

edited

Loading

mtomov commented Nov 15, 2016 •

edited

Loading

mtomov commented Dec 1, 2016 •

edited

Loading

mtomov commented Dec 5, 2016 •

edited

Loading

mtomov commented Dec 8, 2016 •

edited

Loading