Skip to:
Content

Opened 8 months ago

Closed 6 months ago

#7625 closed defect (bug) (fixed)

Nouveau - Nested forms on group create steps

Reported by: hnla Owned by: hnla
Milestone: 3.0 Priority: high
Severity: major Version:
Component: Templates Keywords:
Cc:

Description (last modified by hnla)

Transposed from:
https://github.com/buddypress/next-template-packs/issues/168

Again we see nested forms, nested forms are not allowed.

The group invites backbone template serves the friend member search elements, the create steps form wraps all screens in the create.php file.

This will take some figuring on how to correct due to the many template parts involved here.

When we get to the invites screen step which includes a member search implementation we get this issue of nested forms due to calling an include part for the search. This is tricky to resolve and maybe the simplest solution is to drop the include and hardcode the search form to the create.php template but position outside the main form, or maybe & preferably reducing the steps and work at the creation steps stages and simply placing a message on last screen step saying members can be invited now by visiting ...

Attachments (1)

7625.patch (2.8 KB) - added by hnla 6 months ago.
New create steps invite screen approach

Download all attachments as: .zip

Change History (17)

#1 @hnla
8 months ago

  • Description modified (diff)

#2 @DJPaul
8 months ago

  • Milestone changed from Awaiting Review to 3.0

#3 @DJPaul
7 months ago

@hnla To clarify - the issue is on the "invite friends" screen at the end of the group creation wizard steps?

#4 @hnla
7 months ago

@DJPaul Yep, it's a bit of a frustrating one. it essentially means re-thinking the whole approach to building these steps and the includes, we can't re-use the invites due to it's own form wrapper. Personally I'm in favour of just ditching the invites form/step as one step too many, speed up the creation process, leave a message on last screen that now they can access group and invite members.

#5 @DJPaul
7 months ago

So does existing Legacy not nest forms on this screen?

#6 @hnla
7 months ago

No, iirc it's due to the change in approach on Nouveau to backbone template for invites and the then belief that the template could be re-used without seeing the consequence. As I recall when I looked I couldn't see an easy or relatively quick way of re-factoring hence the frustration and notion to just pull the step altogether, but I can look again, maybe introduce a non backbone template or directly embed the invites into group create steps.

#7 @hnla
6 months ago

re-visited to see what could be done, started looking at unlinking the steps creation tabs to remove the invite tab then removing the template call for step screen, this failed on fact that Nouveau uses a check on BP function bp_is_last_group_creation_step() to determin the correct button i.e 'finish' so would need to alter that function or create new Nouveau version.

Second approach decided to simply use bp_is_group_create() to wrap the backbone section displaying the search form to remove the search form - the object causing the issue - and allow the basic users friend loop list to run, seemed a good approach until I found that the backbone js is more complex as it creates a new nav item when a user is selected and this just simply fails in the group creation screens, likely never worked?!

Did consider calling a new template part for invite steps informing user as to what they could do by way of inviting, but then realised this was a pointless screen, better to not have at all?

So... on hold again while pondering this pita.

@hnla
6 months ago

New create steps invite screen approach

#8 @hnla
6 months ago

Best I can come up with for the moment is a new take on this issue.

Patch adds new template include part to replace the callback for the Nouveau backbone template which doesn't work in this context of create steps.

Adds a new template feedback message to the array and calls that back in the new include.

This approach provides the same screen as we already have with friends listed in a simple loop for selection using bp_new_group_invite_friend_list() selections made here have invites sent on creation steps completion.

In the new group invites menu where we have 'pending invites' we do correctly show invites that have been sent from the creation steps screen, a good thing!

#9 @DJPaul
6 months ago

Devil's advocate: does it *matter* technically if we have a nested form? Does in the inner form break the outside, or something?

#10 @hnla
6 months ago

Hi devils advocate - It's still a prohibited structure, nesting a form element means that the submits cannot properly know to what action they attach to, there are views and technical work around iirc there's an attr don't quote but something like 'form-target' but it's not considered safe.

This, though, appears a moot point as regardless the backbone template is designed to function in one context only so I can't understand why it was added as the callback for the create step screen where it fails to run correctly - unless I have missed something needed to allow it to function correctly?

Version 0, edited 6 months ago by hnla (next)

#11 @hnla
6 months ago

Has to be said that I'm not overly keen on the approach in the patch, but every other approach seems to have issues and complexities to unravel.

#12 @DJPaul
6 months ago

@hnla Patch looks a good, sensible and simple approach.

Only thing I'm going to ask before you commit it, is about the filename of the new template: _create-invites-step.php. None of our other main templates in Legacy/Nouveau have that leading underscore. Maybe just create-invites.php?

#13 @hnla
6 months ago

@DJPaul Agreed I wondered why I added that underscore too, other than somewhat used to as a convention for marking files as partials, happy to change. In addition I had reservations about the template just sort of floating free in that directory, seemed orphaned and out of place, but not sure one odd file justified creating a new directory under ?? /common/ as it's only ever going to be referenced once.

#14 @hnla
6 months ago

<aside> hmm doing what I promised myself I wouldn't do but thinking further about the use of the BP function in this context and given the time I might have fetched the getter function and rebuilt as a Nouveau function but rendering user avatars, another day perhaps.</aside>

#15 @hnla
6 months ago

I will commit this later along with the file name change.

#16 @hnla
6 months ago

  • Owner set to hnla
  • Resolution set to fixed
  • Status changed from new to closed

In 11807:

Nouveau: group create invites

Commit addresses an issue where the invites step of Groups create presents two fundamental problems. In including the backbone JS template partial we include a search form which becomes nested in the main group create form & the backbone JS creates unique nav menu tabs which only function in context of users/groups screens & so invite user selection fails.

Commit adds a new include file which renders the bp_new_group_invite_friend_list() function, replaces the Nouveau JS template callback on the invites step with path to this new file and adds a new screen message to message array to explain screen invites step.

Props DJPaul, hnla

Fixes #7625

Note: See TracTickets for help on using tickets.