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

docs: split up into multiple files and clarify examples #324

Merged
merged 6 commits into from
Aug 6, 2022

Conversation

LordSimal
Copy link
Contributor

@LordSimal LordSimal commented May 28, 2022

Relates to #323 and #136

@LordSimal LordSimal changed the title how to use closure to filter belongsToMany association docs: how to use closure to filter belongsToMany association May 28, 2022
@codecov
Copy link

codecov bot commented May 28, 2022

Codecov Report

Merging #324 (48b6587) into master (1994e97) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #324   +/-   ##
=========================================
  Coverage     94.13%   94.13%           
  Complexity      224      224           
=========================================
  Files            19       19           
  Lines           580      580           
=========================================
  Hits            546      546           
  Misses           34       34           

Help us with your feedback. Take ten seconds to tell us how you rate us.

@raul338
Copy link
Contributor

raul338 commented May 28, 2022

This sounds like I used on #249
It can be done using the value filter

$manager->value('category_id', [
    'multiValue' => true,
    'multiValueSeparator' => ',',
    'field' => ['Categories.id'],
    'beforeProcess' => function (Query $query, array $args, $filter) {
        $query->innerJoinWith('Categories');
    },
]);

so in the URL you can pass category_id=1,2,3,4 (and I belive an array too)

@LordSimal
Copy link
Contributor Author

Indeed this does work as well.
But in my opinion your solution is thinking quite "out of the box" just with the docs given currently.
Especially for newer people who are not that accustomed to the framework.
Or is there a benefit to your approach?

@raul338
Copy link
Contributor

raul338 commented May 29, 2022

My code is combining two filter config in the docs beforeProcess and Value Filter

The code serves as an example of the value filter.
For the callback I would suggest the example would make use of the other parameters

@LordSimal
Copy link
Contributor Author

I didn't mean that I don't understand your example 😉
I was just saying that especially for newcomers a little code block explaining how to filter HABTM associations works can go along way instead of having them reading through all filters and options to let them figure it out themselves.

Since the docs/README.md is already quite big I wouldn't mind splitting it up into several pages, so something like

  • Basic Configuration
  • All default filters incl. examples
  • Additional docs (like everything after the Optional Fields in the current Readme)

@raul338
Copy link
Contributor

raul338 commented May 30, 2022

I agree it would be better to show examples, or repeat titles/sections like it's done on the crud plugin documentation

docs/README.md Outdated Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
@ndm2
Copy link
Contributor

ndm2 commented May 31, 2022

Since the docs/README.md is already quite big I wouldn't mind splitting it up into several pages

All the options on themselves can be quite overwhelming, so a restructuring of the docs would be quite nice I think, dedicated sections for all filters, each with an example would be very helpful especially for new users.

@LordSimal
Copy link
Contributor Author

This is my first version of the split up doc including the more simpler, explicit filter examples:
https://github.com/LordSimal/search/tree/how-to-use-closure/docs

@LordSimal LordSimal changed the title docs: how to use closure to filter belongsToMany association docs: split up into multiple files and clarify examples Jun 4, 2022
@LordSimal
Copy link
Contributor Author

So are there any other objections if my approach of splitting up the docs is good/bad?

----------

`Value` to limit results to exact matches
<details>
Copy link
Member

Choose a reason for hiding this comment

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

The details are collapsed by default and I don't like the fact that an additional click is required to view the code e.g.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since most of these code snippets are only one liners I can actually remove that collapsible element.

@ADmad ADmad merged commit e9c5aa7 into FriendsOfCake:master Aug 6, 2022
@LordSimal LordSimal deleted the how-to-use-closure branch August 6, 2022 18:34
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.

4 participants