Skip to:
Content

Opened 4 years ago

Last modified 6 weeks ago

#2086 assigned enhancement

Some slugs need to be changeable, add language strings.

Reported by: sulley Owned by: johnjamesjacoby
Milestone: 2.1 Priority: normal
Severity: normal Version:
Component: Rewrite Rules Keywords: needs-refresh
Cc: email@…, aesqe

Description

Hello,

I've noticed that some slugs can't be changeable with custom names. That's a major issue with non-english BP installations. Maybe add a language strings for them?

Some of these slugs:
activity -> just-me
activity -> favorites
activity -> mentions

profile -> public
profile -> edit
profile -> change-avatar

messages -> inbox
messages -> sentbox
messages -> compose
messages -> notice

friends -> myfriends
friends -> requests

groups -> mygroups
groups -> invite

Thanks :)

Best Regards

Attachments (4)

2086.01.patch (13.3 KB) - added by boonebgorges 2 years ago.
2086.02.patch (12.2 KB) - added by boonebgorges 2 years ago.
2086.03.patch (14.4 KB) - added by boonebgorges 2 years ago.
2086.04.patch (13.4 KB) - added by johnjamesjacoby 2 years ago.
Iterations

Download all attachments as: .zip

Change History (27)

comment:2 21cdb4 years ago

  • Cc email@… added

comment:3 aesqe4 years ago

  • Cc aesqe added
  • Component set to Core

comment:4 boonebgorges3 years ago

+10000 from me.

It's a big job, as there are lots of places where this happens in the codebase. Will anyone volunteer to work up a first pass at a patch? I will try sometime in the next few weeks if no one else does, but it will distract me from other delightful BP tasks :)

comment:5 cnorris233 years ago

  • Keywords dev-feedback added; slugs translation language file removed

I started to work up a patch for this, but then the thought of BP 1.4 cropped up in my mind. I believe that jjj has alluded to the fact that what he's learned with the bbPress plugin will be applied to BP 1.4. That being the case, it will be much easier and more accessible to change/localize slugs after 1.4. With the broad scope a patch for this ticket would require, it almost seems fruitless. Besides, it's been this way since the beginning, so what's one more release? ;) I would be happy to work up a patch if it's deemed fruitless.

comment:6 DJPaul3 years ago

  • Milestone changed from 1.3 to 1.4

comment:7 DJPaul3 years ago

  • Milestone changed from 1.6 to Future Release
  • Severity set to normal

comment:8 boonebgorges3 years ago

  • Milestone changed from Future Release to 1.6

I would personally like to get this taken care of. It will be tedious, but easy, props for someone. It doesn't even have to be done all at once - patches taking care of single slugs would be fine.

Suggested strategy for maximum flexibility: Build template tags that conditionally define a constant, and then echo and return a filtered version of the constant. Eg:

function bp_messages_compose_slug() {
  echo bp_messages_get_compose_slug();
} 
  function bp_messages_get_compose_slug() {
    if ( !defined( 'BP_MESSAGES_COMPOSE_SLUG' ) ) {
      define( 'BP_MESSAGES_COMPOSE_SLUG', 'compose' );
    }

    return apply_filters( 'bp_messages_get_compose_slug', BP_MESSAGES_COMPOSE_SLUG );
  }

The bit where you define the constants is probably not strictly required (could just use a filter) but we do it with other slugs so we could probably stick to the strategy for ease of documentation. Then, comb through, find the hardcoded references, and replace as necessary.

I'm going to move back to the 1.6 so that people can easily see the ticket and contribute if they want some Mad Props during this dev cycle, with the understanding that we can and should punt if no one patches.

comment:9 DJPaul3 years ago

I'm against adding (what would be) a lot of constants; it's going to clutter things up and possibly make futurepat harder

comment:10 boonebgorges3 years ago

OK. What about just filtering the default string then? It's easy enough to write functions to filter them, and someone could fairly easy whip up a plugin that could filter them all in one fell swoop.

comment:11 boonebgorges2 years ago

  • Keywords has-patch needs-testing added
  • Owner set to boonebgorges
  • Status changed from new to assigned

OK, I've got a patch that demonstrates the core functionality, along with an implementation for the Activity component. (Don't want to spend the time to do the rest of the components before getting the nod that this approach makes sense.) Here are the main pieces of the puzzle:

  • In BP_Activity_Component::setup_globals(), I define an array of slug defaults.
  • Then they're passed through bp_parse_component_slugs(). That function parses the default slugs from the component with those stored in the bp-slugs option. This will allow users to define their own slugs and save them to the db (interface not built yet)
  • Slugs are then passed back to setup_globals() and passed along to BP_Component::setup_globals() and added at $bp->activity->slugs. Along the way they're put through the bp_activity_slugs filter.
  • When setting up nav items, use $this->slugs[slug_name] rather than the current hardcoded slugs.
  • Then, when testing bp_is_current_action(), and when building URLs throughout the component, use the new template functions bp_get_activity_mentions_slug(), etc. Those in turn make a call to bp_get_slug(), which puts everything through a single filter, for maximum plugin flexibility (and minimum direct calls to the $bp global in template tags).

So, this all works quite nicely. Should be totally backward compatible, because plugins that are currently providing hardcoded slugs are hardcoding their links/bp_is_current_action() checks anyway.

Looking for general feedback, as well as some thoughts on a couple questions:

  • What to do about an admin interface? In an ideal world, I think that the current Pages and Components panels should be combined, and slug stuff added to that combined page, with slug settings appearing alongside the associated component. That would take a ton of work, of course. In theory, no admin interface is required for BP 1.6 - the values are all filterable, and if someone really wanted an admin UI, you could do it with a pretty simple plugin. (Paul, maybe I could write something and you could slip it into BP Labs for the 1.6 cycle.)
  • In order to maintain backpat - with our own code as well as with third-party components - I have left $bp->activity->root_slug and $bp->activity->slug in place throughout, instead of switching over to $bp->activity->slugsroot_slug?, etc. It's a bit redundant, but I think it's necessary for now.
  • Related, I currently pass the root_slugs along with the rest of the slugs to the filters. However, since root_slugs come from the associated WP pages, they shouldn't be changed in this method - it would break stuff, probably. It'd be possible to have post-filter enforcement of the root_slug == $bp->pages->{$component}->slug equivalency, but I'm not sure whether it's necessary right now. (In our eventual admin, I'd grey-out the root slugs and provide a link to the edit.php page for the associated WP page.)

Feedback welcome. It'd be nice to get this basic infrastructure into the trunk in the next week or two so that it can be put through its paces. As I say above, the Admin UI could be punted.

boonebgorges2 years ago

comment:12 boonebgorges2 years ago

2086.02.patch is the same thing, with a couple of half-implemented-then-abandoned ideas taken out.

boonebgorges2 years ago

comment:13 DJPaul2 years ago

Wow. Ok, some initial thoughts:

  • The slug array could be e.g. array( $slug_key => array( $slug, $description= ) ). Could be useful for putting an optional description or hint to use in the UI.
  • I've nothing really against template tag functions, but I'd like to vote for using something like: bp_activity_get_slug( 'key' ), which then calls bp_get_slug( 'activity', $key ) in the core component. Seems a bit less verbose wrt. implementation?
  • Probably best to do the bp_parse_component_slugs() stuff in the BP_Component class itself.

Will think about the UI.

comment:14 boonebgorges2 years ago

Thanks for the helpful feedback, Paul!

The slug array could be e.g. array( $slug_key => array( $slug, $description= ) ).

Great idea. I've made the change. It required making the parsing logic a bit more complex (needed to wp_parse_args at a lower level) but it seems to work nicely. It's set up currently so that plugins would be able to filter these values if desired, and they could also be saved in the database (if that was needed for some reason), but they'll always fall back on those provided in the component constructor.

I'd like to vote for using something like: bp_activity_get_slug( 'key' )

I'm kind of indifferent here. If we really want to make it simple, why not just use bp_get_slug( 'activity', $key ) throughout? I personally prefer fewer template tags, but I know that we have a convention of setting things up so that you never have to pass an argument to a function inside of a template file. I didn't change anything in 03.patch, but I can do whatever if there's a general consensus. It's trivial to swap out.

Probably best to do the bp_parse_component_slugs() stuff in the BP_Component class itself.

Good thinking. I was trying to keep BP_Component clean (so I've left the _parse_ function in the global scope instead of making it into a method of BP_Component), but you're right that this should be handled there.

See 03.patch.

boonebgorges2 years ago

johnjamesjacoby2 years ago

Iterations

comment:15 johnjamesjacoby2 years ago

Attached patch changes 'slugs' to 'action_slugs' and removes the root_slug and slug from the array. Also adds a default slug to bp_get_slug()

I think this approach is sound, but per IRC chat with Boone I think it needs more work. With all the looping and functions that are happening here, I think BP_Slug needs its own class with its own methods to prevent some of this from breaking down over time, particularly for third-party plugins that won't be using the new API's.

comment:16 boonebgorges2 years ago

  • Keywords 1.7-early added
  • Milestone changed from 1.6 to Future Release

It's too late to do this right for the 1.6 cycle. Will take care of it right away for 1.7.

comment:17 djpaul22 months ago

(In [6157]) Removing bp_form_slug_conflict_check() for 1.6. See #2086

comment:18 boonebgorges21 months ago

  • Keywords 1.7-early removed

comment:19 DJPaul13 months ago

  • Component changed from Core to Rewrite Rules
  • Milestone changed from Future Release to 1.8
  • Priority changed from major to normal

Should look at this for 1.8

comment:21 boonebgorges11 months ago

  • Milestone changed from 1.8 to 1.9
  • Owner changed from boonebgorges to johnjamesjacoby

comment:22 r-a-y6 months ago

  • Milestone changed from 1.9 to 2.0

comment:23 DJPaul6 weeks ago

  • Keywords needs-refresh added; dev-feedback has-patch needs-testing removed
  • Milestone changed from 2.0 to 2.1

Pretty sure that this ticket will get superseded by the custom rewrite rules going into 2.1, so moving the milestone so it can be reviewed come 2.1.

Note: See TracTickets for help on using tickets.