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

Adds support for value => label definitions in the admin filter options #107

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

maxchirkov
Copy link

Hi John,

I never created a pull request on github before, so hopefully I didn't screw anything up.
So, I implemented the solution I described in my last comment here:
#87 (comment)

I wrote a test too, but for some reason the mockBuilder would not execute my method and always returned null, so I had to instantiated the Extended_CPT_Admin::class.

Feel free to make adjustments or reject if this doesn't feel right to you.

Thanks for the consideration.

@johnbillion
Copy link
Owner

I forgot to say thanks for the PR. I won't have much time to look into this soon but I'll get to it when I can!

@lmartins
Copy link

lmartins commented Oct 3, 2018

Looking forward to this merge as I'm facing the same issue, where I need the filter value to be a numeric value.

@ibes
Copy link

ibes commented Apr 15, 2019

I am running in the same problem that was the reason for the pull request.
What would help to get the issue fixed?

@ibes
Copy link

ibes commented Apr 15, 2019

the pull request works well for me.
Thanks

@johnbillion johnbillion added this to the 4.3.0 milestone Apr 20, 2019
@johnbillion
Copy link
Owner

I need to check whether there's a chance this breaks other functionality that unintentionally passes an associative array of values, but generally I agree this change makes sense.

@johnbillion johnbillion modified the milestones: 4.3.0, Future Aug 9, 2019
@johnbillion johnbillion modified the milestones: Future, 5.0.0 Jul 8, 2020
@johnbillion johnbillion modified the milestones: 5.0.0, Future Mar 27, 2021
@danielpost
Copy link

@johnbillion Do you have an idea when this would be considered? I'm running into the exact same issue as others in the associated issue.

@JUVOJustin
Copy link

@johnbillion if only the docks are missing would it be able to merge? I would be happy to send you a markdown doc with the adjusted section regarding callback functions.

@vinkla
Copy link
Contributor

vinkla commented May 29, 2024

@johnbillion, is there anything I can do to get this merged or have a similar implementation approved? I'm here to help 🙋

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

Successfully merging this pull request may close these issues.

7 participants