Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MNTP does not allow setting media type, or member type when you change to a matching node type of Media or Member in v8.1 #5983

Closed
tom-ingeniuus opened this issue Jul 23, 2019 · 7 comments · Fixed by #6177

Comments

@tom-ingeniuus
Copy link

Using version 8.1 in the latest Chrome browser

I want to create a MNTP picker to select only a particular media type which I have created. When you change the node type to media and then try to select the allowed media types, the selector still shows document types. It should show media types. The same issue occurs if you select a member type nodes, you still get a list of document types, not member types.

Interestingly, when selecting the root node for media, it does display media folders. The root node correctly gets hidden when you select a member type node.

@nul800sebastiaan
Copy link
Member

Ah shoot.. we forgot to test this with media / member types @kjac ! The cause is in this PR: #5506

@kjac
Copy link
Contributor

kjac commented Jul 23, 2019

Shoot. I'll have a look.

@kjac
Copy link
Contributor

kjac commented Jul 23, 2019

So I've been digging into this, and it's interesting to say the least 🤔

Obviously it's an error that you get to select content types when you're configuring an MNTP for members or media. That needs to be fixed one way or another; I'll get back to that later. First to the interesting parts.

Media picker

The new V8 media picker implementation does not support the filtering function required by MNTP to dim non-allowed types. This used to work in V7 with the V7 style media picker. See the media picker controller - there is no usage of or reference to $scope.model.filter, which is where the media type filter is defined (it is however passed into the model as expected).

Member picker

The member picker doesn't seem to have been supporting filtering for a long, long time. Here's a GIF from V7.15; the "Member picker" property is an MNTP configured to select only members of type "Other":

member-picker-with-filter

I have tracked the lack of filtering down to this bit of code. The member picker filtering is "advanced" (meaning it's a custom function by the contentpicker controller here).

So what to do?

We can do one of three things here:

  1. Reinstate the MNTP filter definition prior to V8: Use a picker to select allowed types for MNTP #5506, which was just a textstring. This is super easy to do but it doesn't really solve much.
  2. Amend V8: Use a picker to select allowed types for MNTP #5506 so it supports picking media and member types as well, and then submit new issues to have filtering supported by the media and member pickers. I have a working implementation of this ready for a PR.
  3. Only make it possible to select content types; the type selector would simply display an info text stating that you can only limit by types for content pickers. The implementation I have to option 2 could easily be tweaked for this purpose.

I suggest option 2 as the best cause of action here. @nul800sebastiaan thoughts?

@nul800sebastiaan
Copy link
Member

Hey @kjac - I see I totally forgot to answer this! 🙈

Yes, definitely go for option 2!

@umbrabot
Copy link

Hi @tom-ingeniuus,

We're writing to let you know that we've added the Up For Grabs label to your issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly PR team bot :-)

@kjac
Copy link
Contributor

kjac commented Aug 22, 2019

I'll get on it 😀

@kjac
Copy link
Contributor

kjac commented Aug 22, 2019

@nul800sebastiaan PR in #6177. MNTP property editor support for the configured "allowed types" should be implemented later on (see the PR for details).

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

Successfully merging a pull request may close this issue.

4 participants