We need to add a "Truncation length for 'Reason' field" setting on the global settings page. As discussed in IRC, this setting is currently found in Userpoints Node and Comments. We'd like to move it to the main Userpoints module so that all UP-related modules can use it.

--Ben

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Status: Active » Needs review
FileSize
4.76 KB

Here is a patch.

Note that this works a bit different than what currently have for userpoints_nc:

- The setting from NC only applied to content titles. This one applies to the whole string. Therefor, I increased the default to 70 characters. This has the following advantages:
+ It is automatically applied to all reasons. NC and other modules providing points don't need any special code anymore
+ If it is a link, the title attribute contains the full description and is displayed when you hover over the link.
+ The reason is only shortened in listings not on the details page.

- Because of the last point, I've added the setting to the "Listings" category.

Note that reasons provided by userpoints_nc will still be shortened directly for that module before this applies. So it won't work as you'd expected it when it is set to the same or higher length then the current userpoints_nc setting. Either set this to a lower value or the userpoints_nc one to a very high value.

Once this is in, we can remove the setting from userpoints_nc.

BenK’s picture

Status: Needs review » Needs work
FileSize
23.63 KB

Berdir,

You added some really great functionality here. It's generally working quite well. There are just some odd edge cases that have surfaced and need addressing:

1. If a reason field has an apostrophe in it (perhaps because a node title contains an apostrophe), then special characters are not being escaped properly in the title attribute (which is displayed when hovering). For instance, I had a node title with the word "Ben's" in it, and it displayed when hovering as

Ben'

2. I'm wondering if the title attribute (which displays when hovering) should be an option on the global config page (right after the truncation length). The reason is that if you have a long truncation length set, then it merely displays what is already shown in the reason field on every transaction (and you don't really need it). But if your truncation length is short, it is a great feature. Actually, I have one other possibility for a setting: Only display the title attribute if the truncation length is shorter than the particular Reason field. Thoughts?

3. I'm getting some strange results with the truncation length set to 40. See the first screen shot. On the transaction detail page, it is causing the "Operation" field label to become part of the link on the "Description (Auto Generated)" field above it. The "Description (Auto Generated)" field shouldn't be truncated on the details page, right?

4. On the transaction list page, with the truncation length set at 40, it is causing the "Approved" of the status column to become part of the link in the Reason field. See screenshot #2.

--Ben

BenK’s picture

FileSize
68.62 KB

Screenshot #2 attached...

Berdir’s picture

Status: Needs work » Needs review
FileSize
4.99 KB

Nice catches!

1. This should be fixed now, the title attribute was incorrectly escaped twice.

2. What about just making that the default behavior (only display title if it is shortened). The patch does that now. Not sure about adding too many options.

3. This happened because it was incorrectly truncated without removing html tags before. This messed up the markup. Should work fine now.

4. This was the same issue but the fix is different: since we are on the details page, it shouldn't truncate the reason in the first place :)

BenK’s picture

Status: Needs review » Needs work

Just finished testing the latest patch. Items #1, #3, and #4 are all fixed. Thanks!

Just one problem remains on #2: I like your idea to make the default behavior be to only display the title if it is shortened. Currently, this is working well if you have a manually entered description field. But if you don't have a manually entered description field on a particular transaction (and the auto-generated description is showing instead) then the title seems to always display even if the field is not being truncated.

--Ben

Berdir’s picture

Status: Needs work » Needs review
FileSize
4.99 KB

Found the issue, the problem was that it still checked if the description needs shortening with the html tags. So it thought that it needs to shorten but never actually did.

This should be good now :)

EDIT: This is the old patch, ignore :)

Berdir’s picture

FileSize
5.23 KB

Correct patch.

BenK’s picture

Status: Needs review » Reviewed & tested by the community

Tried out the latest patch and it is working great. All issues are solved. This is RTBC!

--Ben

Berdir’s picture

Status: Reviewed & tested by the community » Fixed

Great, commited!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.