[RFC]Quote link to quoted message

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.2/Rhea branch. Everything listed in this forum will be available in phpBB 3.2.
User avatar
brunoais
Registered User
Posts: 958
Joined: Fri Dec 18, 2009 3:55 pm

Re: [RFC]Quote link to quoted message

Post by brunoais » Sun May 24, 2015 9:49 pm

JoshyPHP wrote:
  • Backward compatibility proved to be too complicated so instead of modifying the original quote_username_open template fragment, it uses a new, separate template fragment named quote_extended which contains XSL for the logic.
That seems fine to me. I think there shouldn't be any issues with that.
JoshyPHP wrote:
I think the date should be on the right of the rest of the quote information. IMO, it should be (I do not include the text such as the "L_WROTE"):
{urlToSource {author}} {date}
OR
{sourcePostLink(icon)} {urlToAuthor {author}} {date}
OR
{sourceLink(externalLinkIcon)} {urlToAuthor {author}} {date}

If it can be confusing if the author link can have two different things, then use these instead:
{sourcePostLink(icon)} {urlToAuthor {author}} {date}
OR
{sourceLink(externalLinkIcon)} {author} {date}
OR
{sourceLink(externalLinkIcon)} {urlToAuthor {author}} {date}

Opinions?
JoshyPHP wrote:
  • The post link contains a small bit of inline JavaScript that replaces its full URL with an anchor link if the post exists on the current page. If someone has a better solution, I'll gladly accept PRs against my repository.
I like the concept. I don't like that it is inline. I think it should be external js and you should use a data-* attribute on it.
Then, develop javascript that searches for all tags with that data-* attribute and execute the check and edit on it. If you'd rather have me doing it, just tell me and I'll make a proposal.

User avatar
JoshyPHP
Registered User
Posts: 349
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC]Quote link to quoted message

Post by JoshyPHP » Sun May 24, 2015 10:04 pm

brunoais wrote:I think it should be external js and you should use a data-* attribute on it.
What would be the upside though? Moving that code to an external file means either adding a bunch of listeners (one on every link) or one listener that will trigger on every click made anywhere in the topic. It's going to execute some code on every page load regardless of whether the user clicks any links, and it will be active only after the page is loaded. It will require more code and be less efficient with it.

User avatar
JoshyPHP
Registered User
Posts: 349
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC]Quote link to quoted message

Post by JoshyPHP » Mon May 25, 2015 2:07 am

JoshyPHP wrote:Edit: one test fails on Travis's PHP 5.3 with the following error message. PHP 5.3 has been removed from my Linux distribution and the test passes on my local version (PHP 5.3.29-pl0-gentoo) so debugging it is especially challenging.
DateTime::setTimezone(): Can only do this for zones with ID for now
Looked into this bug. I can't reproduce locally and the test runs fine on Travis when it's run apart from the rest of the test suite. It's probably an interaction with some other test in the suite, I have no idea. If I try mocking phpbb\user::format_date() the test suite commits sudoku. I may very well just yank the test and pretend it never existed. Disregard that.

User avatar
brunoais
Registered User
Posts: 958
Joined: Fri Dec 18, 2009 3:55 pm

Re: [RFC]Quote link to quoted message

Post by brunoais » Mon May 25, 2015 8:21 am

JoshyPHP wrote:
brunoais wrote:I think it should be external js and you should use a data-* attribute on it.
What would be the upside though? Moving that code to an external file means either adding a bunch of listeners (one on every link) or one listener that will trigger on every click made anywhere in the topic. It's going to execute some code on every page load regardless of whether the user clicks any links, and it will be active only after the page is loaded. It will require more code and be less efficient with it.
The loss of efficiency goes only to the DOM search. The number of listeners is the same.
Besides, this is an useful piece of code that can be use in many different places, not just the link in the quote tag.

User avatar
JoshyPHP
Registered User
Posts: 349
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC]Quote link to quoted message

Post by JoshyPHP » Mon May 25, 2015 4:18 pm

Then I guess you should write it and see with Nicofuma whether he wants it in.

Nicofuma
3.2 Release Manager
3.2 Release Manager
Posts: 297
Joined: Sun Apr 13, 2014 1:40 am
Location: Paris

Re: [RFC]Quote link to quoted message

Post by Nicofuma » Tue May 26, 2015 6:56 am

I like it ;)
Member of the phpBB Development-Team
No Support via PM

User avatar
brunoais
Registered User
Posts: 958
Joined: Fri Dec 18, 2009 3:55 pm

Re: [RFC]Quote link to quoted message

Post by brunoais » Tue May 26, 2015 8:17 am

Which thing, Nicofuma?

Nicofuma
3.2 Release Manager
3.2 Release Manager
Posts: 297
Joined: Sun Apr 13, 2014 1:40 am
Location: Paris

Re: [RFC]Quote link to quoted message

Post by Nicofuma » Tue May 26, 2015 9:32 am

the data-* attribute. That's something we could use in other places.

Implemented by https://area51.phpbb.com/phpBB/viewtopi ... 23&t=48026 and deployed on area51
Member of the phpBB Development-Team
No Support via PM

Post Reply