? Pending

User tests: Successful: Unsuccessful:

avatar ciar4n
ciar4n
17 Dec 2016

Pull Request for Issue #12789 .

Summary of Changes

Removes the edit button from article list and adds an inline edit icon on hover.

Do we really need the edit icon??

Testing Instructions

Apply patch and navigate to Content -> Articles

Current
edit5

Initial
edit2

Documentation Changes Required

Yes

avatar ciar4n ciar4n - open - 17 Dec 2016
avatar ciar4n ciar4n - change - 17 Dec 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 17 Dec 2016
Category Administration com_content Templates (admin)
avatar ciar4n ciar4n - change - 17 Dec 2016
Labels Added: ?
avatar schnuti
schnuti - comment - 17 Dec 2016

? A micro step but still a step.
I did this for my own item lists a long time ago. I personally though prefer to always show the icon (if editable) for 2 reasons.

  1. If I'm new to Joomla, I get a hint where to click to edit an item.
  2. If I know Joomla, I do not need this hint at all.

Probably some CSS is missing for "my breakpoint", Win10, Firefox full screen, Screen 1366x768, left menu minimized

edit-icon

avatar ciar4n
ciar4n - comment - 17 Dec 2016

@schnuti Strangely I am unable to replicate this issue (Win 10, FF). Clear cache maybe? Fetching the latest commit? Are you using the Joomla! Patch Tester?

avatar schnuti
schnuti - comment - 17 Dec 2016

@ciar4n
Sorry, after clearing the browser cache it works as described. Yes, I use the Patch tester. Normally, when I edit CSS files and reload the page, the edited CSS is loaded. Not here obviously?
I'll add the test result!

avatar schnuti
schnuti - comment - 17 Dec 2016

I have tested this item successfully on 07ce54b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13250.

avatar schnuti schnuti - test_item - 17 Dec 2016 - Tested successfully
avatar Bakual
Bakual - comment - 17 Dec 2016

Can we use a class that is more intuitive? indic-edit is a bit short if you don't know what it does. indicate-edit would be better imho.

avatar ggppdk
ggppdk - comment - 17 Dec 2016

I tested a little different CSS:
to add a small padding to the table cell 4px, to make a little more room to it (till reaching the previous cell)

and thus display the edit-icon via toggling its "display" (while having position: absolute;) instead of changing its margin-left/right

That would avoid shifting the title on hover which some people may find annoying
... just the above will ovelap with "checked-out" icon (when this is shown), maybe also move checked-out icon after title would be acceptable ?

avatar ciar4n
ciar4n - comment - 17 Dec 2016

@Bakual Thank you. Amended as suggested.

If this is considered the way to go should the same be applied to all edit buttons (moduleList, pluginList etc.)?

Also as suggested by @schnuti , should the icon be always displayed?..

edit1

@cpfeifer

avatar Bakual
Bakual - comment - 17 Dec 2016

Maybe it is a stupid question but Is it possible to do the whole thing without the itemList-title class? Asking because it would be much more reusable for other cases.

avatar ciar4n
ciar4n - comment - 17 Dec 2016

@Bakual Sure. Certainly makes more sense. Now targeting a[data-original-title="Edit"] instead.

@ggppdk It would depend on moving the check-in tag after the title. If not the edit icon will appear next to the check-in tag which may be a bit deceiving? Presuming I understand you correctly you are suggesting the following ('Sample Article 2' hovered)...

edit2

avatar ggppdk
ggppdk - comment - 17 Dec 2016

Yes exactly what you show in the picture,

Also another argument to move the lock to the end:
is that it looks a little ugly to have some titles shifted to the left (or right for RTL) when the lock is shown

I find the picture that you posted great )) !!

avatar dgt41
dgt41 - comment - 17 Dec 2016

@ciar4n @ggppdk you can move the lock icon to the column status, checkout at the end is an item status ?

avatar ciar4n
ciar4n - comment - 17 Dec 2016

@dgt41 Something like this?...

edit3

avatar Bakual
Bakual - comment - 17 Dec 2016

If you move it to the status, try to integrate it into the existing buttons. Having a separate one looks ugly to me.

avatar ciar4n
ciar4n - comment - 17 Dec 2016

@Bakual Adding to the btn-group with the current styling..

edit4

avatar dgt41
dgt41 - comment - 17 Dec 2016

@ciar4n another idea is to hide the grouped (publish-feature-dropdown) if the item has needs checkout (lock icon is shown) and just have the locked button in their place... (clicking on the lock icon forces a page refresh so the grouped icons will show up)

avatar sovainfo
sovainfo - comment - 17 Dec 2016

Pleas document how to suppress the inline edit icon as well !

avatar ciar4n
ciar4n - comment - 17 Dec 2016

@dgt41 Nice idea!

Now looking like this...
edit5

avatar ciar4n ciar4n - change - 17 Dec 2016
The description was changed
avatar ciar4n ciar4n - edited - 17 Dec 2016
avatar Bakual
Bakual - comment - 17 Dec 2016

Pleas document how to suppress the inline edit icon as well !

Since it is done using CSS in ISIS only, it would be as simple as just overriding those rules. But what issue are you seeing with it?

avatar sovainfo
sovainfo - comment - 18 Dec 2016

Anticipate a lot of requests to remove it, obviously it depends on the implementation. But, what I have seen so far make me want to suppress it. Convinced that the most basic feature of internet (hyperlink) is understood by everyone and there is no need for additional ways to accomplish that.

avatar dgt41
dgt41 - comment - 18 Dec 2016

@sovainfo add in custom.css (isis):

.indicate-edit {
display:none;
}

That should do it!

avatar ciar4n ciar4n - change - 18 Dec 2016
The description was changed
avatar ciar4n ciar4n - edited - 18 Dec 2016
avatar ciar4n
ciar4n - comment - 18 Dec 2016

Personally I would agree with @sovainfo. The hyperlink itself should be enough to indicate it's purpose. From a styling perspective, the icon adds nothing and would be considered annoying by most.

avatar schnuti
schnuti - comment - 18 Dec 2016

Please do not forget that the main advantage is that the edit toolbutton is removed.

@sovainfo As I wrote before, I agree but as I'll have to override the CSS anyway, to support one line toolbar for "normal screen width and height (1024x768?)", I hope I can remove the hover effect as well. Buttons got even larger in 3.7 and two more might appear.

avatar infograf768
infograf768 - comment - 18 Dec 2016

If we take off the Edit button, I personally find it nice to have the edit icon.

avatar schnuti
schnuti - comment - 18 Dec 2016

@infograf768 - Fixed or hover?

avatar infograf768
infograf768 - comment - 18 Dec 2016

Hover.

avatar sovainfo
sovainfo - comment - 18 Dec 2016

@schnuti Would go for the idea of icons only. Consider the idea too easily dismissed in the other thread.
Only stated it is bad and inconsistent, no backing up for those statements!
Very strange that the icons need a label at the top, but not in the list. Talking about consistency!
Consistency doesn't mean the same regardless the circumstances. Standard practice is to provide the label as tooltip. Archive and Batch are probably the only two needing that, you could decide to provide labels for those.

avatar mbabker
mbabker - comment - 18 Dec 2016

Please don't remove the edit button until the whole issue with clicking the title ending up with that message about not being able to edit the item because of the session issue is fixed.

avatar dgt41
dgt41 - comment - 18 Dec 2016

@mbabker is the current behaviour any different?

avatar mbabker
mbabker - comment - 18 Dec 2016

If you get that message you can hit the check box and edit button in the toolbar (see one of Phil's explanations about it and the whole form submission POST request versus clicking the title sending a GET request).

avatar ggppdk
ggppdk - comment - 18 Dec 2016

Please don't remove the edit button until the whole issue with clicking the title ending up with that message about not being able to edit the item because of the session issue is fixed.

Right , we forgot of this

@dgt41
in some server the redirection that happens when clicking on the edit links is cached (due to bad server configuration typically in .htaccess)

but if user checks the checkbox and clicks the EDIT button then the form submits and the form submission (POSTed) is not cached by browsers, thus editing works

avatar mbabker
mbabker - comment - 18 Dec 2016

If you want to fix it without #10753 (which the more I see the more I feel like it's a hack in an already hacked together system), hijack the edit link with JavaScript and cause it to trigger the form submission.

avatar dgt41
dgt41 - comment - 18 Dec 2016

@mbabker @ggppdk we can use javascript and interrupt the default behaviour (event.preventDefault) and pass it through the form submission...

avatar ggppdk
ggppdk - comment - 18 Dec 2016

If you want to fix it without #10753 (which the more I see the more I feel like it's a hack in an already hacked together system), hijack the edit link with JavaScript and cause it to trigger the form submission.

@mbabker
yes despite working pretty well, it is a hack, and now that you and @dgt41 suggested,
it may be good idea to do this:

we can use javascript and interrupt the default behaviour (event.preventDefault) and pass it through the form submission...

I mean do it regardless of this PR

avatar dgt41
dgt41 - comment - 18 Dec 2016

@ggppdk I've tried couple of times before to solve the unneeded check-in, by restricting (or informing) the user that they should use the toolbar buttons to exit a singular form view: #4773, #5815
I think solving the (UX mostly) problem should been done there, not hacking the list view.
But I'm willing to put some line in the list view, if that's what's decided...

avatar ggppdk
ggppdk - comment - 18 Dec 2016

I've tried couple of times before to solve the unneeded check-in, by restricting (or informing) the user that they should use the toolbar buttons to exit a singular form view

The above is a different case, mbabker did not mean the locked (checked-out) records,
he meant this: #10753

-- usually happening because of browser caching the redirection of edit links (1st URL calculates ACL, then redirected to 2nd URL that shows the form)


So preventing form window close / back button clicked / etc

-- which is not easy (not possible to be done via JS due to browser security)
-- is not relevant here, here speaking to do a different thing

This case is much simpler,

  • we just want to prevent the article link from being followed
    e.g. by attaching an onclick event handler with:
    stoppropagation / preventdefault, or by doing this:
<a onclick="return false;" ... href="...">

And then besides preventing the link from being followed,

  • also get the form to post as if edit button was clicked:
    (i added new lines for being easier to read)
onclick="
jQuery(this).closest('tr').find('input[name=\'cid[]\']').prop('checked', true); 
Joomla.submitbutton('article.edit'); 
return false;" 
avatar ciar4n
ciar4n - comment - 19 Dec 2016

Closing. Can be reopened if clicking title issue is resolved.

avatar ciar4n ciar4n - change - 19 Dec 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-12-19 00:39:08
Closed_By ciar4n
avatar ciar4n ciar4n - close - 19 Dec 2016

Add a Comment

Login with GitHub to post a comment