Skip to:
Content

BuddyPress.org

Opened 6 years ago

Last modified 2 months ago

#8001 new task

Review at.js integration for updates and/or swapout

Reported by: boonebgorges's profile boonebgorges Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Core Keywords: needs-testing has-patch
Cc:

Description

Previously: #7928

According to https://github.com/ichord/At.js/commit/a42b3182a9433506eabf8aaaa5eeb653561a27e7, At.js is no longer maintained. We should explore swapping out with a maintained package. https://github.com/zurb/tribute is recommended, but I have no idea how different the API is.

If we maintain the library, we could monkey-patch fixes like https://github.com/ichord/At.js/issues/321#issuecomment-319516017

Attachments (5)

8001.tribute01.diff (268.3 KB) - added by dcavins 6 years ago.
Use Tribute for @-lookups of users.
8001.02.diff (139.3 KB) - added by dcavins 4 years ago.
Update original patch; add support for BP Nouveau.
8001.02.patch (139.3 KB) - added by imath 4 years ago.
Same than 8001.02.diff except that it has been generated using --no-prefix
8001.03.patch (144.4 KB) - added by imath 4 years ago.
8001.04.patch (145.1 KB) - added by dcavins 4 years ago.
@imath's patch + exclude self from search query and improve text contrast of suggestions for accessibility/legibility

Download all attachments as: .zip

Change History (27)

#2 @dcavins
6 years ago

I'm attaching a patch that introduces Tribute in favor of At-who. I'm not sure that this is a clear improvement (it trades one kind of complexity for another). Thoughts:

  • The division of labor is that the library watches the inputs we specify for the trigger @ and then compares the string after the @ to the values (user objects) you passed to it. It sorts the best match to the top of the list and handles keyboard and mouse interaction with the results list. Our wrapper handles the remote AJAX lookup and returns the fetched user objects to Tribute.
  • This library is large (48KB transfer and ~200KB unzipped) and does a lot more than we really need, like fuzzy matching from a list of possibilities. Our most typical case is a set of AJAX-fetched options in response to a query, so everything is a match. The only time that the fuzzy matching is used in our case is when the user is interacting with the pre-constructed friends list (generated by php and stored in the JS object BP_Suggestions.friends), which happens rarely.
  • Tribute gives each match a score to order the results. It would be cool to influence those results so that your friends are always on top. (Then we could also stop pre-caching friends in php.)
  • I was not able to get Tribute to work with the TinyMCE editor window. It seems possible, but I ran out of will in this session. (Also, what about Gutenberg blocks?)
  • I ran into some issues within Tribute where I thought I could pass options in to change the behavior and output, but the promising sub module options were hardcoded in the main module, so things that looked like options actually weren't. I imagine Zurb would consider accepting pull requests that were reasonable. (They'd actively like help with improving how the library works with TinyMCE, for instance.)

So, Tribute is promising, but maybe overkill for what we need (and not as flexible/easy to work with as we'd like). Let me know what you think.

@dcavins
6 years ago

Use Tribute for @-lookups of users.

#3 @boonebgorges
6 years ago

Thank you for working on this!!

It does sound like Tribute is a bit more than what we need today, but I like the idea that it's a well-maintained library that has features that we could one day leverage.

One key question I've got is: Does moving to Tribute fix some of the outstanding @-mention interface bugs that have come up in the last year? If it does, then it seems like a good move.

Regarding TinyMCE - can you remind me where in BP we supported the atwho interfare in TinyMCE? Was in in the messages area? Or in the Dashboard itself? (Side note that the swap-out breaks messages in Nouveau, which for some reason lists atwho directly as a dependency, rather than our wrapper)

#4 @dcavins
6 years ago

Re TinyMCE: With the atwho method, you can @mention users in post content and such from the WP dashboard. (It doesn't cause anything to happen, like a notification or a profile link, the user is just mentioned in the text of the post like "I had a nice chat with @username about BuddyPress".) I'm dubious about how useful it is, but I think we should be able to recreate it using Tribute, so that we don't lose a feature that someone loves dearly.

I'm not sure about the outstanding @-mention interface bugs. The only issues I knew of were accessibility problems, and, at first blush, Tribute does seem to resolve those (I'd love for @mercime to have a look).

I will look into handling the change in Nouveau as well, and will produce a new patch.

In other news, WP just started using at-who and caret on the support forums: https://meta.trac.wordpress.org/changeset/8064

#5 @imath
4 years ago

  • Milestone changed from Up Next to 7.0.0

#6 @imath
4 years ago

  • Milestone changed from 7.0.0 to Up Next

Let's work on this early during next development cycle.

#7 @imath
4 years ago

  • Milestone changed from Up Next to 8.0.0

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


4 years ago

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


4 years ago

#10 @dcavins
4 years ago

I've attached a new patch that updates the original patch. It also adds support for the "To" field in BP Nouveau private messages and uses the BP REST API for the suggestions lookup. Here are some notes about the patch:

  • No one has shared a solution to getting Tribute to work with TinyMCE. The only place we would wish to use Tribute with TinyMCE is in the private messages "content" field in BP Nouveau.
  • With Gutenberg, WP has introduced a username autocompleter in the post content editor (based on jQueryUI autocomplete I think), so we no longer need to add a mentions watcher there.
  • With this patch, the mentions watcher is used in the "to" field in BP Nouveau and in the activity post body.
  • The BP REST API lookup works great! One functionality difference is that the old wp-ajax endpoint included non-activated users (those with no "last activity") in the results, but the REST API excludes them. (I think that behavior is more consistent with BP overall.)

I guess my doubts are that this is a big library to provide two little bits of functionality, but the "to" field of messages really needs autocomplete. Mentions in activity items are reported to the user, so that is also useful.

Let me know what you all think.

(Here's some info about the autocompleter used in Gutenberg: https://developer.wordpress.org/block-editor/components/autocomplete/)

Last edited 4 years ago by dcavins (previous) (diff)

@dcavins
4 years ago

Update original patch; add support for BP Nouveau.

#11 @imath
4 years ago

  • Keywords has-patch added

Thanks a lot for your work on it @dcavins 👍 I'll test it asap

@imath
4 years ago

Same than 8001.02.diff except that it has been generated using --no-prefix

#12 @dcavins
4 years ago

Thanks, @imath. You should have pinged me to --no-prefix the patch, I just forgot, ha ha.

@imath
4 years ago

#13 @imath
4 years ago

@dcavins I've just tested your patch. It works! Thanks a lot for your work on it.

In 8001.03.patch​ I'm suggesting some improvements:

  1. Let's use sass to generate the mention.css style and let's improve the style so that it looks closer than what we had so far 😉
  2. Let's avoid some jQuery deprecated stuff ( $(document).ready() and $.isArray )
  3. Let's avoid arrow functions ( users => cb( users ) ) or if we want to use Modern JS, let's do it completely 😁
  4. There was a Tribute warning, so I've added some code to only attach Tribute when not already attached to an input.

In Nouveau it's a bit annoying we can't have the autocomplete to work into the WP Editor as it's currently working with At.js. I haven't tried to add it yet, but I believe we should try.

I also believe we can still improve it trying to avoid the current user to be into the autocomplete.

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


4 years ago

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


4 years ago

@dcavins
4 years ago

@imath's patch + exclude self from search query and improve text contrast of suggestions for accessibility/legibility

#16 @imath
4 years ago

Awesome! Thanks a lot for your updated patch @dcavins I’ll give it a look asap. I see what you were meaning about tinyMce. I haven’t found a way yet, it’s pretty annoying.

I’m wondering if we should keep the at/who libraries and deprecate them, I’m a bit afraid some plugins might be using them ?

#17 @dcavins
4 years ago

@imath, I agree totally. When I first tried Tribute out 2 yrs ago, I thought that, given time, someone would work out the best practice for using Tribute with TinyMCE. It hasn't happened yet, and I think we should not expect it to happen. Tribute seems to be actively maintained, but I wonder if it's the mentions library we should be using? Or, to go totally feature creep on us, should we continue to use TinyMCE, since WP is moving away from it? :)

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


4 years ago

#19 follow-up: @imath
3 years ago

  • Milestone changed from 8.0.0 to Under Consideration

@dcavins I'm sorry I made you work on this and now I'm moving the ticket out of 8.0.0. It's just I cannot resolve myself to feel good with a sort of regression due to Tribute not supporting tinyMce. Sorry again 😬

#20 in reply to: ↑ 19 @dcavins
3 years ago

Replying to imath:

@dcavins I'm sorry I made you work on this and now I'm moving the ticket out of 8.0.0. It's just I cannot resolve myself to feel good with a sort of regression due to Tribute not supporting tinyMce. Sorry again 😬

No problem! It was worth looking into it again.

#21 @espellcaste
4 months ago

  • Milestone changed from Under Consideration to Up Next

Let's come back to this and decide next steps.

cc: @imath and @dcavins

#22 @imath
3 months ago

  • Milestone changed from Up Next to 15.0.0

#23 @espellcaste
2 months ago

  • Milestone changed from 15.0.0 to Awaiting Review
Note: See TracTickets for help on using tickets.