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

Reopening pull request #2065 forum userlist wrapping #5309

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rica-carv
Copy link
Contributor

Reopening issue #2065

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist

Removed hardcoded html and use wrapping instead, for userlist shortcode...
@Jimmi08
Copy link
Contributor

Jimmi08 commented Oct 7, 2024

@rica-carv
I appreciate your work so now I am trying to understand what you did.

This PR will be never approved. forum shortcodes file is already updated and there is a conflict in it with your code.

In the original PR @CaMer0n already told you that it breaks old themes. My objections are that you didn't fix this shortcode fully.

Link to user profile should use e107::getUrl() way, not hardcoded HTML code. On one place you are removing it, but on other place you let it there.

global $listuserson; - should be $listuserson = e107::getOnline()->userList();

return; should be return "";

user link should be:


		$uparams = array('id' => $oid, 'name' => $oname;
		$link = e107::getUrl()->create('user/profile/view', $uparams);

Everything is already managed (new way) in online plugin
It should be done similar way as in $ONLINE_MENU_TEMPLATE['default']['online_members_list_extended']

I hope it helps somehow. The original issue should be opened, because this should be fixed. Thanks

Next notes:

  • online users under forum should be all users or only users on forum?
  • online user area detection fails because of SEF-URLs. Online_location is used as e_REQUEST_URI but all checks are against URL format forum.php, news.php etc.

@rica-carv
Copy link
Contributor Author

@Jimmi08 Sorry, but i don't understand your post. If you check both files i changed, i only take out hardcode html inside the forum_shortcodes.php file to forum_template.php file!
I didn't do anything else...

All the changes you're speaking were already there, so i don't see any other conflict....

I think you're mixing up old change with my new ones.... and i don't understand the changes you're talking about....

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.

2 participants