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 filters to sort & select based on subvalues #5622

Merged
merged 9 commits into from Jun 14, 2017

Conversation

Projects
None yet
6 participants
@gnuletik
Contributor

gnuletik commented Dec 3, 2016

Hi,

I was trying to sort a collections using the sort filter like this {% assign collec = site.my_collection | sort 'header.year' %} but the subvalue wasn't working, so I made a quick fix.

Maybe I should do the same thing for item.data[property.to_s] and item[property.to_s] making a new method.

What do you think about it ?
Thanks !

gnuletik added some commits Dec 3, 2016

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Dec 3, 2016

Member

@gnuletik can also add to the test/test_filters.rb to check if this works as intended, especially with any future changes..

Member

ashmaroli commented Dec 3, 2016

@gnuletik can also add to the test/test_filters.rb to check if this works as intended, especially with any future changes..

@gnuletik

This comment has been minimized.

Show comment
Hide comment
@gnuletik

gnuletik May 16, 2017

Contributor

Hi ! Any news on this ?

Contributor

gnuletik commented May 16, 2017

Hi ! Any news on this ?

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks
Member

pathawks commented May 16, 2017

@parkr

Looks good! Let's use .reduce to clean things up.

Show outdated Hide outdated lib/jekyll/filters.rb
Show outdated Hide outdated lib/jekyll/filters.rb
@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli May 17, 2017

Member

@gnuletik I ran a couple of tests locally using your sample json file and page, above against both manually creating a test site and the following:

      should "return sorted by subproperty data array" do
        site = fixture_site
        site.data.merge!({
          "array" => [
            { "a" => { "b" => 2 } },
            { "a" => { "b" => 1 } },
            { "a" => { "b" => 3 } },
          ]
        })
        assert_equal(
          [
            { "a" => { "b" => 1 } },
            { "a" => { "b" => 2 } },
            { "a" => { "b" => 3 } },
          ], @filter.sort(site.data["array"], "a.b")
        )
      end
    end

My inference is that you don't have to patch item.respond_to?(:data) for this feature to work with data files

@pathawks @parkr pinging you guys if could confirm my finding when possible

Member

ashmaroli commented May 17, 2017

@gnuletik I ran a couple of tests locally using your sample json file and page, above against both manually creating a test site and the following:

      should "return sorted by subproperty data array" do
        site = fixture_site
        site.data.merge!({
          "array" => [
            { "a" => { "b" => 2 } },
            { "a" => { "b" => 1 } },
            { "a" => { "b" => 3 } },
          ]
        })
        assert_equal(
          [
            { "a" => { "b" => 1 } },
            { "a" => { "b" => 2 } },
            { "a" => { "b" => 3 } },
          ], @filter.sort(site.data["array"], "a.b")
        )
      end
    end

My inference is that you don't have to patch item.respond_to?(:data) for this feature to work with data files

@pathawks @parkr pinging you guys if could confirm my finding when possible

gnuletik added some commits May 18, 2017

@gnuletik

This comment has been minimized.

Show comment
Hide comment
@gnuletik

gnuletik May 18, 2017

Contributor

@ashmaroli I did the tests too and your're right : patching item.respond_to?(:data) doesn't implement this feature for data files. I reverted the changes. So I don't know when it's being used.

Contributor

gnuletik commented May 18, 2017

@ashmaroli I did the tests too and your're right : patching item.respond_to?(:data) doesn't implement this feature for data files. I reverted the changes. So I don't know when it's being used.

@gnuletik

This comment has been minimized.

Show comment
Hide comment
@gnuletik

gnuletik May 19, 2017

Contributor

So, is this version okay ?

Contributor

gnuletik commented May 19, 2017

So, is this version okay ?

@gnuletik

This comment has been minimized.

Show comment
Hide comment
@gnuletik

gnuletik Jun 14, 2017

Contributor

up

Contributor

gnuletik commented Jun 14, 2017

up

@parkr

parkr approved these changes Jun 14, 2017

@parkr parkr changed the title from Accessing subvalue from filters to Allow filters to sort & select based on subvalues Jun 14, 2017

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 14, 2017

Member

@jekyllbot: merge +minor

Member

parkr commented Jun 14, 2017

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit e031ac9 into jekyll:master Jun 14, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

jekyllbot added a commit that referenced this pull request Jun 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment