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

WIP: Dynamic OG complex field #161

Closed
wants to merge 29 commits into from
Closed

Conversation

RoySegall
Copy link
Collaborator

No description provided.

@amitaibu amitaibu changed the title Dynamic OG complex field 101. WIP: Dynamic OG complex field Mar 7, 2016
@RoySegall
Copy link
Collaborator Author

@amitaibu I think i'm on to something here:
create_group_content___site-install

@pfrenssen
Copy link
Collaborator

Oh boy, looks like this will invalidate all my work on OgComplexWidgetTest :) Never mind, this looks like a great improvement!

@RoySegall
Copy link
Collaborator Author

👍

@RoySegall
Copy link
Collaborator Author

I was pretty amazed to get to this point. I think the multiple autocomplete widget is the bad boy in this story since we need to wrap it with add another item but i'll handle it.

@RoySegall
Copy link
Collaborator Author

I made another improvement to the API. Some minor documentation:

    dpm(OgGroupAudienceHelper::getAvailableWidgets());
    OgGroupAudienceHelper::setWidgets('node', 'group_content', 'og_group_ref', [
      'default' => 'options_select',
      'admin' => 'options_select',
    ]);

@amitaibu
Copy link
Owner

amitaibu commented Mar 9, 2016

nice! now only to see if you can save and edit ;)

@pfrenssen
Copy link
Collaborator

This is actually not interfering at all with the OgComplexWidgetTest from #140, this is even answering some questions I had in there :)

The relevant part is the testFieldModes() test, that actually tests this functionality. I will take it out of #140 and move it here. That way this is instantly tested, and it also reduces the scope of #140.

@pfrenssen
Copy link
Collaborator

@RoySegall OK if I work on the test part of this? I won't touch your code.

@RoySegall
Copy link
Collaborator Author

I have a couple of things i'd like to do you could join in such as ordering the code.

I'll ping you.

@RoySegall
Copy link
Collaborator Author

I tried to do a couple of things but you can join in although I think it would be very confusing.

@pfrenssen
Copy link
Collaborator

OK no problem, I'm going to work on #162 today.

@RoySegall
Copy link
Collaborator Author

Didn't got the time but all the default groups selection are working. We need to handle the other group.

Also we need to handle views selection handler but we can keep for it later.

if ($handler instanceof EntityReferenceAutocompleteWidget) {
// No need for extra work here since we already extending the auto
// complete handler.
return $multiple;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

return $widgets;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for splitting this off as static methods in OgGroupAudienceHelper? Are you planning on reusing them somewhere else?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - we would like to get all the available widget so the user could pick them through the UI. No?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Way to go! That's the spirit!

@amitaibu
Copy link
Owner

@RoySegall can you please give a quick update on what you accomplished yesterday.

@RoySegall
Copy link
Collaborator Author

Yeah. Sure. The handlers give us the correct groups - my groups and other groups. There are still some WTF with the other group widget when it's set to autocomplete and when selecting groups via autocomplete the groups are being ignored - problem with FAPI and how the values are processed.

Except that - we all set to go :)

@RoySegall
Copy link
Collaborator Author

Update: I managed to have a single auto complete element and get his selected group. The next part will be to have a multiple one but I think that in this case we would to create our own 'Add another button' as we did at the beginning. I'll update.

@RoySegall
Copy link
Collaborator Author

I pushed now a small work on the custom widget - https://gfycat.com/DisloyalRegularKodiakbear - looks good for now.

@amitaibu
Copy link
Owner

@RoySegall what's the status on this one - what's left?

@RoySegall
Copy link
Collaborator Author

I need to handle "Add another button" for the other groups widget.

@RoySegall
Copy link
Collaborator Author

Closing infavor of #248.

@RoySegall RoySegall closed this Jul 9, 2016
amitaibu added a commit that referenced this pull request Sep 4, 2016
Prevent notice on create new content
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants