Skip to:
Content

Opened 7 years ago

Closed 8 months ago

#787 closed enhancement (fixed)

All profile field data becomes searchable links--is irrelevant for some data types

Reported by: jeffsayre Owned by: boonebgorges
Milestone: 2.5 Priority: normal
Severity: normal Version:
Component: Extended Profile Keywords: has-patch needs-testing commit
Cc: vivek@…

Description

When viewing a user's profile data, all fields that are not URLs, get turned into searchable links.

Whereas I like the idea of making data searchable via links, this implementation is flawed. At a minimum, it should not be linking radio buttons or checkbox selections--what value does that offer? The data in these types of fields is often too specialized to be of much meaning in a search.

Also, it arbitrarily excludes phrases over 5 words long. I believe site admins (maybe even users) should have the option to not have profile field data made into searchable links. Finally, clicking on one of these links can be confusing to users as it sends them to the members homepage without explaining what is going on. Only in very large sites will any relevant results be returned.

Here is where this happens:

In profile-loop.php (member theme), the template tag bp_the_profile_field_value() is called. This invokes the filter xprofile_filter_link_profile_data(), located in bp-xprofile-filters.php.

Obviously, making every link clickable is not necessarily the proper action--even if the goal is to facilitate more universally searchable data. Certain fields do not led themselves to being searchable simply by virtue of the data they contain.

Attachments (5)

x-profile.patch (27.6 KB) - added by sooskriszta 11 months ago.
Patch - 1st cut
x-profile.patch.1.0.1.diff (7.5 KB) - added by LenLay 11 months ago.
New patch version. Added help text to tooltp and added fixes to admin part.
x-profile.patch.1.0.1.2.diff (7.0 KB) - added by LenLay 11 months ago.
Small fixes. I cleaned unnecessary changes
787.patch (1.7 KB) - added by mercime 8 months ago.
787.2.patch (1.7 KB) - added by mercime 8 months ago.

Download all attachments as: .zip

Change History (40)

#1 @jeffsayre
7 years ago

A better phrasing of the issue:

Whereas in general I like the idea of making data searchable via links (it's sort of like tags), this implementation is flawed. At a minimum, it should not be linking the user's "Full Name" field as the best search result for that is for the viewer to stay exactly where they currently are--on that member's profile.

Also, linking radio button groups or checkbox selections seems unnecessary. The data in these types of fields is often too specialized to be of much meaning in a search. Furthermore, the code arbitrarily excludes phrases over 5 words long. Finally, clicking on one of these links can be confusing to users as it sends them to the members homepage without explaining what is going on. Only in very large sites will any relevant results be returned.

It might be nice if there was an administrative option to not make profile fields clickable.

#2 @jeffsayre
7 years ago

A perfect example of the seemingly random and confusing nature of this filter can be found in this forum thread. Make sure to visit the link provided as it demonstrates the point quite well.

http://buddypress.org/forums/topic.php?id=3190

#3 @apeatling
7 years ago

  • Milestone set to 1.1

Would love a patch!

#4 @apeatling
7 years ago

  • Milestone changed from 1.1 to 1.2

Moving this to 1.2, not enough time before release.

#5 @plrk
7 years ago

I remember I wrote some bp-custom.php code to remove this from all fields but checkboxes, radiobuttons, and multiselectors. You can find it here: http://dropbox.jobjorn.se/profile_links.txt

#6 @johnjamesjacoby
7 years ago

  • Milestone changed from 1.2 to Future Release
  • Owner set to johnjamesjacoby
  • Status changed from new to assigned

Moved out of 1.2 and into future release to be grouped together with extended profile field component update.

#7 @cnorris23
6 years ago

  • Component set to XProfile
  • Keywords needs-patch added

#8 @sooskriszta
2 years ago

  • Severity set to normal

Copying from #5612

BuddyPress automatically links all text in user profile fields between commas, e.g. See profile page http://csillamvilag.com/modellek/modell/angelaveress/
In some cases this is pretty cool - if I want to find all female models, I can just click on and that leads me to search results page http://csillamvilag.com/modellek/modell/?s=Nő
However, there are cases where links are expected to behave differently, e.g. if I see a phone number linked (+3630/450-01-90 on this profile), I expect there to be a tel: link behind it, not a link to search page for that telephone number http://csillamvilag.com/modellek/modell/?s=%2B3630%2F450-01-90
The same would be true for emails (as a user, I'd expect mailto:) and URLs.
Further, sometimes, the linking is just weird and annoying, such as (in the same profile) Célom. Just because something is between commas (or is in the beginning of a paragraph and has a comma after it), does not mean it should be automatically searchable.
With this in mind, it seems best that the admin of the site should have control over which fields shall be clickable to a search results page.
To start off, textarea type xprofile fields may be made non-clickable by default.

#9 @sooskriszta
2 years ago

  • Cc vivek@… added

#10 follow-up: @boonebgorges
2 years ago

  • Keywords ux-feedback added
  • Milestone changed from Future Release to 2.1
  • Priority changed from minor to normal
  • Type changed from defect (bug) to enhancement

This is a longtime annoyance that we should be able to make some progress on for BP 2.1. I think that a full overhaul of this feature is going to take a lot of work, but maybe we can break it down in a way that is spread over versions (or at least is not so overwhelming). Some thoughts:

  1. We should come up with intelligent defaults for each field type. Dates, for instance, should almost certainly be set to false (and actually, are already hardcoded as an exception in xprofile_filter_link_profile_data()). If someone could make a list of field types and make initial recommendations about default states, that would be helpful.
  2. Currently the linking logic is like this: Split the string based on the placement of commas. (If a string is longer than 5 words but has no commas, bail.) For each comma-separated item, if it's less than 5 words, make it clickable. The two primary results of this logic are (a) 1-5 word entries are made clickable automatically (think: "Chicago" for "City"), and (b) "lists" of items are kinda sorta parsed (think: "foo, bar, baz, barry"). Arguably, the behavior in (a) is acceptable; the behavior in (b), while a good idea, is not - it leads to many false positives and false negatives, and it's unreasonably English-centric. So we might want to drop the (b) behavior (at least by default).
  3. Ideas are welcome regarding where to put the admin setting, and what it should look like.
  4. Ideally, there would be a nice UI that would allow users to make any phrase in a field linked to a directory search. (Think of it like Twitter hashtags, but without requiring the # symbol.) Years ago I wrote a plugin http://wordpress.org/plugins/custom-profile-filters-for-buddypress/ that introduces square brackets for this purpose (think: "I enjoy [cheeseburgers] and [fried chicken]"). This works, but is not, by itself, good UX. To build a real interface for this (which would, I imagine, involve selecting text with the mouse and clicking some sort of button) will take some design work, and some fancy javascript to implement. Ideas/mockups welcome.

#11 @sooskriszta
2 years ago

Radical(ish) suggestion:
For 2.1, let's turn it OFF for single fields (text, textarea, number, date, url) and ON for multi-fields (checkboxes, radio buttons, dropdowns, multi-selects). I feel that multi-fields are really where this feature is useful anyway. This logic actually does not require string-parsing as far as I can see....because we are dealing with closed-ended questions, thus this essentially deals with individual field record values.
2.2 - we can keep the above settings as default, and add a checkbox next to each multifield, asking admin whether the field should be linked or not. For text fields, let's actually do hashtags (why not?). People understand the concept pretty well, thanks to Twitter et al.

#12 follow-up: @DJPaul
2 years ago

Lots of interesting ideas. For 2.1, I suggest:

  • we add a simple checkbox to the xprofile field UI, along the lines of "link key phrases to profile search".
    • But store the actual option in xprofile meta as a negative option. e.g. 'dont_link_to_search'.
    • Discoverability is v important and should remain the default in majority of cases, and as such, we don't want to store a lot of "yes" value for each field.
  • add a property to the field type class to set a default; allow searches on everything except date, number, url.
    • I don't think we should touch the other older field types because I could see arguments for setting the default both ways.

This is a little off-topic, but I strongly think hashtags are something BuddyPress should implement soon, perhaps in 2.2, and I'd rather wait until we have that kind of robust system in place before we spend time adjusting the current linking logic and potentially upsetting people.

If we're going to change that CSV-style linking, for example, rather than say "you have to write code to bring this behaviour back", I'd prefer to say to people, "now just use hashtags".

#13 in reply to: ↑ 12 @sooskriszta
2 years ago

Replying to DJPaul:

  • add a property to the field type class to set a default; allow searches on everything except date, number, url.

Default should be off for text, textarea as well IMHO.

#14 @boonebgorges
2 years ago

Default should be off for text, textarea as well IMHO.

I think DJPaul is arguing that we should be conservative for 2.1, in anticipation of a more thorough overhaul in 2.2 (possibly related to hashtags). I think his suggestion is a good one.

But store the actual option in xprofile meta as a negative option. e.g. 'dont_link_to_search'.
Discoverability is v important and should remain the default in majority of cases, and as such, we don't want to store a lot of "yes" value for each field.

Yes, and this technique will have the bonus of making backward compatibility easier, as existing fields will need no migration in :)

#15 @DJPaul
2 years ago

Right.

#16 @DJPaul
2 years ago

  • Keywords ui-feedback early added; ux-feedback removed
  • Milestone changed from 2.1 to 2.2

No patches ever materialised for this, and moving to 2.2 when we should implement the conservative changes at least.

#17 @DJPaul
2 years ago

  • Keywords ui-feedback removed

#18 @johnjamesjacoby
2 years ago

  • Owner changed from johnjamesjacoby to boonebgorges

Related to #5839. Assigning to Boone for review.

Last edited 2 years ago by johnjamesjacoby (previous) (diff)

#19 @DJPaul
21 months ago

  • Keywords early removed
  • Milestone changed from 2.2 to Future Release

#20 @LenLay
11 months ago

  • Keywords has-patch added; needs-patch removed
  • Priority changed from normal to high

Created patch for this ticket
https://www.dropbox.com/s/ek98t1lolq6j5dr/x-profile.patch?dl=0

authors:
@Lenlay, @sooskriszta

#21 @sooskriszta
11 months ago

  • Keywords needs-testing added
  • Priority changed from high to normal

@sooskriszta
11 months ago

Patch - 1st cut

#22 in reply to: ↑ 10 @sooskriszta
11 months ago

Replying to sooskriszta:

However, there are cases where links are expected to behave differently, e.g. if I see a phone number linked (+3630/450-01-90 on this profile), I expect there to be a tel: link behind it, not a link to search page for that telephone number http://csillamvilag.com/modellek/modell/?s=%2B3630%2F450-01-90
The same would be true for emails (as a user, I'd expect mailto:) and URLs.
Further, sometimes, the linking is just weird and annoying, such as (in the same profile) Célom. Just because something is between commas (or is in the beginning of a paragraph and has a comma after it), does not mean it should be automatically searchable.
With this in mind, it seems best that the admin of the site should have control over which fields shall be clickable to a search results page.
To start off, textarea type xprofile fields may be made non-clickable by default.

Replying to boonebgorges:

We should come up with intelligent defaults for each field type. Dates, for instance, should almost certainly be set to false (and actually, are already hardcoded as an exception in xprofile_filter_link_profile_data()). If someone could make a list of field types and make initial recommendations about default states, that would be helpful.

This patch basically does the following:

  1. All multi fields (checboxes, dropdowns, multiselect, radio buttons) are linked by default
  2. All single fields (date selector, number, textbox, textarea, url) are not.

Based on the use cases I am familiar with, that's the "intelligent" set of defaults I could come up with.

#23 follow-up: @boonebgorges
11 months ago

  • Milestone changed from Future Release to 2.5

@LenLay @sooskriszta Thanks very much for the patch. The general approach looks good.

I think we need to rethink naming. To my ears, is_searchable means "do the contents of this field appear in search results". See #4821 for a related discussion. More appropriate is create_search_links or something like that.

Backward compatibility for when these were public methods.

We don't need this for the new method.

Let's have a wrapper function for checking global $field; (bool) $field->is_searchable. That way we can add a filter.

The default settings seem good to me. If we have a filter, people can set it however they'd like programatically.

The UI label description should have some info about the linking works for text fields. It's kinda hard to explain - chunks of text between commas, etc - so if anyone has suggestions, please jump in.

Let's do it for 2.5.

#24 in reply to: ↑ 23 @sooskriszta
11 months ago

Replying to boonebgorges:

To my ears, is_searchable means "do the contents of this field appear in search results". More appropriate is create_search_links or something like that.

Agree 100%

The UI label description should have some info about the linking works for text fields. It's kinda hard to explain - chunks of text between commas, etc - so if anyone has suggestions, please jump in.

What's the convention on help text? Is there a CSS class? Should it be provided as tooltip to a "question mark" icon image?...

#25 follow-up: @sooskriszta
11 months ago

What do you feel about long, explanatory help text?
"If selected then the value on member profile tab will be a link, clicking on which searches for profiles of other members with the same response."

#26 in reply to: ↑ 25 @boonebgorges
11 months ago

Replying to sooskriszta:

What do you feel about long, explanatory help text?
"If selected then the value on member profile tab will be a link, clicking on which searches for profiles of other members with the same response."

The UI in the patch currently has a "description" paragraph that reads:

If selected then the value on member profile tab is a link, clicking on which searches for profiles of other members with the same response.

which is exactly what you've suggested. Coincidence? :)

I think it needs to be more inclusive. If a large chunk of text is auto-linked, then only small chunks of it, determined by an algorithm that looks for chunks of text shorter than 6 words, will be linked. This is pretty hard to explain, so suggestions are welcome.

@LenLay
11 months ago

New patch version. Added help text to tooltp and added fixes to admin part.

@LenLay
11 months ago

Small fixes. I cleaned unnecessary changes

#27 @sooskriszta
8 months ago

Core team, any updates on the patch test?

This ticket was mentioned in Slack in #buddypress by boone. View the logs.


8 months ago

#29 @boonebgorges
8 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 10557:

Introduced 'Autolink' setting for XProfile fields.

Since BP 1.1, XProfile fields have been "autolinked". This means that a user's
entry for profile fields is linked to a search of the Members directory, using
that field's value as a search term. For short values - say, "Wichita, KS" for
the field "City" - the entire field value would be a link to a member search
on the phrase "Wichita, KS". For longer values, as in a multiline text field,
an algorithm detects "lists" (words between commas) and creates links
from them.

This feature was intended to aid in user discovery and serendipity. Sometimes
it works. But sometimes it makes very little sense.

This changeset introduces the ability for administrators to configure this
setting, on a per-field basis.

  • The new Autolink metabox on the XProfile Field Admin panel lets admin enable or disable the feature.
  • Smart defaults are pre-selected for new fields, or fields for which no value has yet to be saved. Namely: "multi fields" like checkboxes and radio buttons have Autolink enabled by default, while "single fields" have it disabled.

Props LenLay, sooskriszta, jeffsayre, boonebgorges.
Fixes #787.

#30 @boonebgorges
8 months ago

Hi @sooskriszta - Thanks for the ping. I dropped the ball on this through 2.5, and only remembered it last night (about 12 hours before your ping!) and started refreshing it. I decided to try sneaking it into 2.5, after some slight changes to the interface and the wording used. See [10557].

#31 @sooskriszta
8 months ago

Cheers @boonebgorges

Sorry I disappeared from the BP 2.5 email tokens sheet...exams et al.

This ticket was mentioned in Slack in #buddypress by boone. View the logs.


8 months ago

@mercime
8 months ago

#33 @mercime
8 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@boonebgorges Attached patch removes an extra label associated with the same select field for the new Autolink metabox, adds screen reader text and for attribute to the label closest to the input field, and changes a punctuation mark.

#34 @DJPaul
8 months ago

  • Keywords commit added

Looks good

@mercime
8 months ago

#35 @mercime
8 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 10584:

Fix labels for the select form control in the new Autolink metabox.

Fixes #787.

Note: See TracTickets for help on using tickets.