invites-loop template re-factoring

[hnla]

Patch re-factors the newish invites loop template for single groups.

Now the send invites screen is more a 'manage invites' one with select from friends list and invited people loop, the messages and titling were a little out of kilter.

Patch adds heading for main screen, and two sub headings for the friends list & the invited list, removing the existing message that was displayed at bottom! of the screen. Also friends list is if/elsed in case a member has no friends to invite and instead displays explanatory message.

Verbiage doubtless can be improved but this lends a little semantic meaning to the sections.

[hnla]

re-factored template

[boonebgorges]

I like the idea of adding some helpful annotation to this otherwise sorta confusing screen. But this needs a bit more thought. The markup for the headings (h1 and h2) is causing flow problems with the floated elements on the screen, and they look huge on certain themes (try Twenty Fourteen to see what I mean). We don't have these kinds of headers on other group subpages, so this creates some inconsistency in layout and info structure.

One thought that might help to clarify a little: changing the 'Send Invites' tab name to 'Invites'.

[hnla]

However headings they are and headings they ought to be sadly what you're describing are theme issues with styles if there are issues with floated elements then somewhere there need to be rules to clear elements of sibling floats and this patch didn't purposfully take into account styling which is a seperate mater. Markup should always be used in a semantic manner it's then a matter for visual styling to decide how it wants to render that to screen. Best way of understanding this and a practise important to follow is to always disable styles to see what the DOM is creating by way of structured markup flow.

I take the point though on what is being seen but sadly to circumvent those issues probably means taking correct elements and changing to something like a paragraph tag but that isn't a correct use they are not paragraphs - or we visually style those new elements to render them smaller. One issue that always will arise with themes though is that of describing headings as it's too easy for heading to be inserted out of sequence upsetting a natural flow.

I guess we can and ought to address a few simple styles if we go with this re-factoring which I can attend to later.

[boonebgorges]

How about not using <h1> and <h2>? We use <h1> for the name of the group in group-header.php, so maybe <h2> and <h3> would be more appropriate. This would also mitigate some of the problems with header size in various themes. This should be tested against, at a minimum, the last four WP default themes; if necessary and possible, we could add a minimal style to bump down the font-size to make it look right across these themes.

[DJPaul]

There is good progress on a patch but it's not close enough or tested enough for it to get into 2.1 unless someone puts some serious effort in during this next week. I think I'd like to see this in 2.2 early, which gives (whoever wants to work on the ticket) a bit more time to make it more perfect.

[hnla]

Yep meant to respond to Boon's last comment of which heading changes make sense, sizing I'm reluctant to do though as encountered too many issues recently with BP font sizes and themes, just too tricky an area to try and second guess.

Needs patching afresh, obviously. I might be able to deal with that early week, if not able will patch and it will have to go in for 2.2.

[hnla]

Re-visiting this and decided what might be a better approach is to patch this in two parts, chiefly because there is a somewhat poor ux in initial send invite screen.php where we check for bp_get_total_friend_count( bp_loggedin_user_id() ) however we then don't do anything if that returns false so if you're a billy no mates then clicking send-invites leaves you on a blank screen with no indication of why.

This initial patch simply adds a 'else:' and message that you need some friends before being able to use this screen.

If nothing else of this ticket gets in in time we might slip this through? Verbiage requires scrutiny though.

I'll try and work on the earlier patch to update and improve.

[hnla]

Patch addresses no message for send-invites.php

[hnla]

Second patch addresses invites-loop.php.

This adds a check for invite friends list returning false - a member can have friends yet those friends for one reason or another ( already members of group) would result in rendering the ul markup without any data so check checks and if false adds a message to the effect that Your friends are currently all members of this group t's probably an edg case that this would occur but did with my test setup so would seem sensible to allow for the possibility?

The other changes are simply to add headings for the screen and for the actual 'Current invites section of the screen.

One concern is, as Boone stated originally, that other group screens aren't provided with headings causing a little inconsistency, although that really is more a fault of the other screens as these headings make sense and would definitely be added under any other circumstance.

The markup for the headings (h1 and h2) is causing flow problems with the floated elements on the screen.

There is an issue here but it's one caused by twentytwelve which has a very odd global ruleset on generic heading elements of 'clear: both' this is an odd rule as it's clearly highly theme specific and not the sort of rule one would ever write, only clearing where specifically required, I have spotted this as an issue before and therefore decided that it ought to be addressed for BP elements so under the BP namespace token '#buddypress' I added a ruleset of 'clear: none' for grouped heading elements as I can think of no reason one would need to be clearing these within bp screen sections.

Headings as suggested were dropped down to h2/h3 however font sizing not addressed as I've noticed a fair few issues lately with our attempts to dictate font sizes running into pretty bad compound errors, my thinking is that BP's only real option is use keywords or rem units here to avoid those problems, but equally not sure BP should be trying to style a major element on behalf of a theme.

[hnla]

Patch invites-loop template

[DJPaul]

These patches need to be refreshed and merged into one patch file.

[hnla]

5509-2 supersedes all previous patches and refreshes & merges them into one.

Patch re-works the approach as described above for checking friends count and 'bp_get_new_group_invite_friend_list' out of the invites-loop include to the parent file, we check if user has friends, and check friends list returns true before calling the invite loop (friends list will return false if user has friends but they are all already members of this group thus causing a possible third condition to exist and to check for)

If we return false on our initial checks we pass over the loop and output one of two messages 'have no friends' or all friends are members.

In the loop if we haven't yet received a selection we display short info message prompting a selection from list, this message removed when selection made and invite user loop is generated for display as per original behaviour.

Patch takes into account Boon's concerns with heading tag and drops to h2.

Minimal styling provided to reduce size of the headings and selector added for message block to our message rules to provide a block wrapper styled as existing message elements.

This is only tested so far in 2014 and needs running through a few more WP standard themes.

[hnla]

re-factored, merged and refreshed patch

[dcavins]

I'm a fan of helpful error messages and enough text so that inexperienced users aren't totally lost.

Some thoughts:

• BP uses a combination of #message.info for other notices, so I think we could use that same structure here for the "no friends" error message and save a new style for themers to catch. Like:

  <div id="message" class="info">
           <p>Sorry, no members were found.</p>
  </div>
  

• The logic to test for no friends has to come before "do I have non-member friends to invite" because having no friends means you also have no non-member friends to invite.

• Appearance - Hmm, the new header font sizing is a bit of a mixed bag. See attached screenshots. Short of sniffing the theme used, I think hnla's done pretty well here.

• Text - I've simplified the strings to these:
  • pane header: "Manage your group invitations"
  • atop the invite list: "Your current invitations"
  • no friends selected: "Select friends to invite."
  • no eligible friends to choose from: "All of your friends already belong to this group."
  • no friends at all: "Group invitations can only be extended to friends. Once you've made some friendships, you'll be able to invite those members to this group."

I've added a new patch that adds my opinions. Thanks for working on this hnla.

[dcavins]

Slight modifications of hnla's recent patch

[dcavins]

Twenty Twelve with modified header sizes

[dcavins]

Twenty Twelve with inherited header sizes

[dcavins]

Twenty Thirteen with modified header sizes

[dcavins]

Twenty Thirteen with inherited header sizes

[dcavins]

Twenty Fourteen with modified header sizes

[dcavins]

Twenty Fourteen with inherited header sizes

[dcavins]

Twenty Fifteen with modified header sizes

[dcavins]

Twenty Fifteen with inherited header sizes

[hnla]

@dcavins Thanks for looking over.

BP uses a combination of #message.info for other notices, so I think we could use that same structure here for the "no friends" error message and save a new style for themers to catch. Like:

My only reasoning here was to introduce a more generic container block to agregate messages, but not to fussed on this point so lets roll without.

Fine with message string changes.

Headings are an issue, I have in fact recently run through the BP styles to re-factor many font-size properties to use keywords as em/% units can have consequences in unknown layouts, so my earlier use of percentage units here might need reviewing.

[r-a-y]

Had a chance to apply the patch.

I think the headings are nice, but they are not dealbreakers.

The most important part of the patch is the work done to bp-legacy/buddypress/groups/single/send-invites.php.

I'm okay with committing that as well as the string changes for 2.2.

IMO, if we are going to add a heading, we only need one. I think the "Your current invitations" heading in the invite loop is sufficient enough.

What does everyone think?

[hnla]

I can understand your point, personally I liked the main heading to the screen as it makes clear we're managing the aspects of sending invites, and would quite like other screens to be equally clear but as you say this is not fundamentally why I approached this task so if you feel strongly to remove then please do so, perhaps someone else might agree that or offer opinion?

[dcavins]

I agree that the big win in this ticket is the improved error messaging. The headings are a nicety. And I think I agree with r-a-y that one should suffice. (I'd leave the main heading.)

After all, the left column and the right column are displaying the same information in different formats--of your eligible friends, who has an outstanding invite?

I'll also echo what hnla says above, that having an honest-to-goodness heading for the pane title improves the page structure, in all cases but especially for screen readers, but I'd like to see that idea applied uniformly rather than a one-off here and there.

[r-a-y]

Thanks for chiming in, hnla and dcavins.

I do appreciate the work that's gone into the send-invites.php template. I think the usability is way better, so I'll go ahead and commit that some time during the beta period and will leave this ticket open for further discussion about the headings.

Also, just wanted to note that we do actually use subheadings on a group's "Manage > Settings" and "Manage > Forum" tabs ("Forum" tab shows up if bbPress is enabled). They both use <h4> for the element, which semantically isn't great, but just wanted to add that tidbit here.

[r-a-y]

In 9357:

bp-legacy: Enhance usability on a group's "Send Invites" page.

This commit adds better feedback messages depending on if a user has
friends to invite to a group.

See #5509.

[r-a-y]

Oops! Forgot to add props in r9357 for hnla and dcavins :(

hnla: Please feel free to create a new ticket specifically for headings in BP templates.

Going to mark this one as fixed. Thanks everyone and apologies again for the missed props!