Skip to content
This repository has been archived by the owner on Mar 8, 2021. It is now read-only.

JavaScript error in Doc Manager (1.1dev) #639

Closed
janniconl opened this issue May 19, 2016 · 9 comments
Closed

JavaScript error in Doc Manager (1.1dev) #639

janniconl opened this issue May 19, 2016 · 9 comments

Comments

@janniconl
Copy link

janniconl commented May 19, 2016

Doc Manager version 1.1

  • Open Doc Manager module
  • Go to 4th tab (Other Properties)
  • Select Publish/Unpublish or Show in Menu/Hide in menu (doesnt matter)
  • Add a range of resource id's, e.g. 2** or 25,25,26
  • Click submit

A JS error is thrown:
TypeError: document.range is undefined for docmanager.js line 81 (https://github.com/modxcms/evolution/blob/develop/assets/modules/docmanager/js/docmanager.js). Anyone else having this error?

@Deesen
Copy link
Contributor

Deesen commented May 20, 2016

When I moved "sort menu-index" to the resource-tree I forgot to correct the tab-index. Should be fixed.

@yama yama closed this as completed in 4956aee May 20, 2016
@janniconl
Copy link
Author

janniconl commented May 23, 2016

Thx for fixing!

Just curious, why did you remove the sort-menu-index-option completely from DocManager? It took me a while to find out you need to right-click a menu option to sort its children. Why not leave it in DocManager and also add it to the resource-tree? Sometimes it is good and user-friendly to have 2 approches to a certain feature.. Especially since the sorting really fits in the DocManager module, the change is not well documented and in light of the "if its not broken dont fix it" principle

@Deesen
Copy link
Contributor

Deesen commented May 23, 2016

I thought it makes most sense to move it instead of duplicating it, see #532. Also you commented on that issue so I´m wondering you missed it?

@janniconl
Copy link
Author

janniconl commented May 23, 2016

I did not miss the feature being implemented, but I did miss the fact that the feature was removed from DocManager completely (I thought adding it to the resource tree was extra, not a replacement)

@fourroses666 mentions "Would that be a replacement of the tree or just extra option?". The answers following gave me the idea it would be an extra option. But I guess I did not read or understand it properly.

But now u mention it, looking at the commit "Sort menu-index: Moved from docManager to tree context-menu " I should have seen it earlier. My bad, sorry.

My thoughts: the DocManager is a powerful tool for admins (as mentioned in #532), I think sorting should be there as well, since it is a logical place to find this feature and it has been there forever. Adding an extra option to the tree, to allow non-admins to sort aswell, is a great idea/feature though!

What do you guys think on this subject?

@bossloper
Copy link
Contributor

I'd be ok with the sort left in DocManager, though I'll probably end up sorting from the menu tree as it requires a lot less clicking to accomplish the same!
A major object of adding the menu-index sort to the menu was to be able to set up so non-admins do not need DomManager access; so allowing hiding DocManager via the module user group access. Mission accomplished - thanks Deesen!

@janniconl
Copy link
Author

I agree with both of your points @bossloper. I will probably use the tree-option mostly aswell, now that I know where to find it.
If no-one else is missing it in DocManager of finds it suitable to leave it there I guess we can just leave it as is then.

@nick0
Copy link

nick0 commented May 24, 2016

I love the sound of sorting in the tree - that is a wonderful addition.
But I agree with @janniconl that you should still be able to sort from the Doc Manager as well.
I vote for both places

@esszett
Copy link
Contributor

esszett commented May 24, 2016

I like the idea, but as its a quite powerfull tool, I would be happy to have a role permission option to switch it off for some users. In worst case scenario a user could mess up a whole site with just one click… and there is no undo available. In recent projects I did hide the menu-index field for editors in certain situations … that could be bypassed by the new tree feature, right?

@bossloper
Copy link
Contributor

@esszett - The 'tree sorting' (menu index sorting) from the right click on the menu tree honours Manager access permissions. So a manager can only sort resource containers (s)he has access to edit. i.e the same permission check used for 'Edit Resource' on the same popup menu.
I would have thought if a manager is given access to add/edit new pages then they can be trusted to sort those pages (in that container)?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants