Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linked and formatted user names #527

Merged
merged 26 commits into from Mar 16, 2014
Merged

Linked and formatted user names #527

merged 26 commits into from Mar 16, 2014

Conversation

Klap-in
Copy link
Collaborator

@Klap-in Klap-in commented Feb 3, 2014

Implements an event which can modify the link below usernames, and the
displayed user name.

When no name supplied, the name of currently logged-in user is used.

Fixes:

  • FS#1558 - Link to user page in function tpl_userinfo
  • FS#2151 - more options for showuseras -- actually the suggested fix is using interwiki links...

2 question left:

  • Howto combine current behavior and a new option for own userlink
  • Howto easily configure the userprofile link?

Implemented as:

  • Add 4th 'showuseras' select option: 'username with interwikilink'
  • FS#2151 interwiki links for configurable links
  • FS#2713 'use existing internal link color for internal interwiki links' is implemented for interwiki links as local wiki links.
  • check usage of <bdi> tags

Implements an event which can modify the link below usernames, and the
displayed user name.

When no name supplied, the name of currently logged-in user is used.
*
* @triggers COMMON_USER_LINK
*/
function userinfo($username = false) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think null or maybe an empty string would make slightly more sense as a default.

@splitbrain
Copy link
Collaborator

One way to deal with the rewriting problem could be handling the user shortcut in a special way, and treating the result as a wiki page. So we would provide user wiki:user:{URL} as default.

Hmm thinking about it, we could always do that when the link does not contain a slash (eg. is not an URL or path). When this is the case we put the resolved link through wl(). This should give us enough flexibility.

@Klap-in
Copy link
Collaborator Author

Klap-in commented Feb 14, 2014

summary sofar:

  • added showuseras option 'username_link' which uses interwiki resolving for link to profile
  • added wrap up with COMMON_USER_LINK event trigger, to replace/extend default user name&link building.

todo's:

  • check <bdi>
  • fix FS#2713 as well? or other PR?
  • check {PATH} placeholder in unit tests.

@@ -24,12 +24,13 @@ amazon.de http://www.amazon.de/exec/obidos/ASIN/{URL}/splitbrain-21/
amazon.uk http://www.amazon.co.uk/exec/obidos/ASIN/
paypal https://www.paypal.com/cgi-bin/webscr?cmd=_xclick&amp;business=
phpfn http://www.php.net/{NAME}
coral http://{HOST}.{PORT}.nyud.net:8090/{PATH}?{QUERY}
coral http://{HOST}.{PORT}.nyud.net:8090{PATH}?{QUERY}
freecache http://freecache.org/{NAME}
sb http://www.splitbrain.org/go/
skype skype:{NAME}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn. I forgot about this usecase. this will break with your change. You no longer can create a shortcut to have special protocol links like skype:, mailto: or javascript:

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could use a (pseudo) full link syntax instead?

user   [[wiki:user:{NAME}]]

But users would expect these to work too then:

user   [[wiki:user:{NAME}|Profile for {name}]]
user   [[wiki:user:{NAME}|{{some:img.png|Profile}}]]
user   [[wiki:user:{NAME}|{{http://example.com|some:img.png|Profile}}]]

which doesn't really make sense. Maybe we can do something like in mime.conf and prefix things that should be resolved as page name with a !

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FS#2713 suggests using _shortcut or .shortcut for internal links, you added !shortcut as well.

Which variant do we like?

@Klap-in
Copy link
Collaborator Author

Klap-in commented Feb 15, 2014

@selfthinker can you have a look at my usage of the <bdi> stuff?

$url = $url.rawurlencode($reference);
$url = $url . rawurlencode($reference);
}
//url without slashes is handled as a pageid
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the current behavior anymore.

freecache http://freecache.org/{NAME}
sb http://www.splitbrain.org/go/
skype skype:{NAME}
google.de http://www.google.de/search?q=
go http://www.google.com/search?q={URL}&amp;btnI=lucky
user :wiki:users:{NAME}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use singular here, so :wiki:user:{NAME}.
Also not sure if the wiki namespace is a good and/or essential place for it. :user:{NAME} would be easiest and is more universal.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. :user:{NAME} seems to be a good default.

@selfthinker
Copy link
Collaborator

The "Logged in as" username link needs to be blue to match the design. I will add that in a bit.

@selfthinker
Copy link
Collaborator

The use of <bdi> is fine.

@selfthinker
Copy link
Collaborator

I'm wondering if there should be an option to remove the link from the user at "Logged in as"? Always adding it will otherwise essentially force every wiki to have user pages...!

@splitbrain
Copy link
Collaborator

huh? there is only a link when you select the appropriate displayas option or is there?

@selfthinker
Copy link
Collaborator

Ah, yes, true. Sorry, my bad.
Because the showuseras option otherwise does not affect "Logged in as", I assumed it would also not affect if it's a link or not. That behaviour is a bit odd and less predictable, but sort of makes sense.

@michitux michitux mentioned this pull request Mar 6, 2014
@splitbrain
Copy link
Collaborator

What's the state here? I think it's good to merge?

@Klap-in
Copy link
Collaborator Author

Klap-in commented Mar 14, 2014

It is ready so far i know.

The user icon is a colored image lib/images/interwiki/user.png. This is nice for the normal wikilinks, but i can image that some prefer a gray image for the times it is used in the header.

An idea is to reuse the profile image of the 'edit profile' link via css, only for user interwikilinks in the header.

  • this shows the profile image twice in some cases.

What do you think?

@Klap-in Klap-in mentioned this pull request Mar 14, 2014
@splitbrain
Copy link
Collaborator

I updated the CSS to override the icon in the header. good to merge in my opinion

👍

if($evt->advise_before(true)) {
if(empty($data['name'])) {
if($conf['showuseras'] == 'loginname') {
$data['name'] = $textonly ? $data['username'] : hsc($data['username']);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does one have hsc() and not the other?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

textonly shouldn't require html escaping.

@Chris--S
Copy link
Collaborator

I don't like the name of the function userinfo() -- too similar to $USERINFO. How about userlink() ?

@Klap-in
Copy link
Collaborator Author

Klap-in commented Mar 16, 2014

The naming is improved as indicated.

Klap-in added a commit that referenced this pull request Mar 16, 2014
Linked and formatted user names
@Klap-in Klap-in merged commit 02fdb91 into master Mar 16, 2014
@Klap-in Klap-in deleted the userlink branch March 16, 2014 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants