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

Allow ordering media by title #295

Merged
merged 6 commits into from Jun 10, 2016

Conversation

Projects
None yet
3 participants
@mfairchild365
Copy link
Contributor

mfairchild365 commented May 10, 2016

This allows ordering media by title. It also re-works the filter and sort nav so that it is re-used and more accessible.

@mfairchild365

This comment has been minimized.

Copy link
Contributor

mfairchild365 commented May 10, 2016

@spennythug I assigned to you for visual feedback. @kabel is welcome to provide feedback too. :)

'datecreated' => [
'label' => 'Recent',
'url' => $context->getURL(array('orderby' => 'datecreated', 'order' => 'DESC'))
],

This comment has been minimized.

@spennythug

spennythug May 16, 2016

Contributor

UGGGGGHHHHH!! WHITESPACE! barfs

This comment has been minimized.

@mfairchild365

mfairchild365 May 17, 2016

Contributor

What whitespace?

This comment has been minimized.

@spennythug

spennythug May 17, 2016

Contributor

GET RID OF IT

</span>
</p>
<?php
$buttons = new stdClass();

This comment has been minimized.

@kabel

kabel May 27, 2016

Contributor

As a best practice, I would not instantiate a stdClass. I consider it an internal PHP class that happens to be usable from userland. Instead, I would use an array and cast it using (object) $buttons.

This comment has been minimized.

@mfairchild365

mfairchild365 May 27, 2016

Contributor

I'm going to do $buttons = (object)[]; and you can't stop me.

This comment has been minimized.

@kabel

kabel May 27, 2016

Contributor

Lol, why mix object and array syntax? You must just like the -> operator

This comment has been minimized.

@spennythug

spennythug May 27, 2016

Contributor

FIGHT THE POWER!

This comment has been minimized.

@mfairchild365

mfairchild365 May 27, 2016

Contributor

I'm all about the -> operator

@kabel

This comment has been minimized.

Copy link
Contributor

kabel commented May 27, 2016

Hey! We can claim this as a feature that YouTube doesn't have 😉

$active_class = $is_current_option ? 'active' : '';
?>
<li>
<a href="<?php echo htmlentities($details['url'], ENT_QUOTES) ?>" class="wdn-button wdn-button-brand <?php echo $active_class ?>">

This comment has been minimized.

@kabel

kabel May 27, 2016

Contributor

Does Savvy not escape this be default? If not, perhaps $savvy->escape(...) would keep it consistent.

This comment has been minimized.

@mfairchild365

mfairchild365 May 27, 2016

Contributor

You are not wrong, and I would like to see this fixed. However I think it is out of scope of the current PR. Savvy in the project has never been configured to escape... and I'm worried about double encoding if I turn it own now without being super careful.

This comment has been minimized.

@kabel

kabel May 27, 2016

Contributor

Eek. We could be rife with injections!

</p>

<?php
$buttons = new stdClass();

This comment has been minimized.

@kabel

kabel May 27, 2016

Contributor

Same thing about best practice with stdClass.

@mfairchild365 mfairchild365 merged commit 6bd115a into unl:4.0_template Jun 10, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment