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

Fix - Allows Label Columns to be Sortable #1256

Merged
merged 5 commits into from
Jul 10, 2023

Conversation

lrljoe
Copy link
Collaborator

@lrljoe lrljoe commented Jun 26, 2023

Presently in testing

This adds the capability for label-only columns to be sortable, by checking first for the Column by Select Name, but if that is null, checks for the column's existence by Slug Name.

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests and did you add any new tests needed for your feature?
  2. Did you update all templates (if applicable)?
  3. Did you add the relevant documentation (if applicable)?
  4. Did you test locally to make sure your feature works as intended?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@lrljoe
Copy link
Collaborator Author

lrljoe commented Jun 29, 2023

Needs some minor further testing before merging.

@lrljoe
Copy link
Collaborator Author

lrljoe commented Jul 4, 2023

As far as I can tell, this is now ready @rappasoft - although I'd appreciate you giving it a quick poke before I merge!

This enables a "label" column to be Sortable (via the callback), presently this does not work.

            Column::make('Name Label')
            ->sortable(function (Builder $query, string $direction) {
                return $query->orderBy('name', $direction); 
            })
            ->label(fn($row, Column $column) => $row->name),

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No change here, just whitespacing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No change here, just whitespacing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Introduction of "isColumnBySlug" which is used to determine if the Column is an actual column based on the Slug, if it can't be found based on field.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This makes an attempt to find the Column based on the SelectName first, but then swaps over to Slug if it can't find the Column.
This allows the Column to be sortable if it has a sortCallback

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This makes an attempt to find the Column based on the Slug

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This fails over to finding the Column by Slug if it can't by SelectName

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This defaults to getColumnSelectName if it is associated with a field. But fails over to finding the Column based on Slug if not.

@rhnkyr
Copy link

rhnkyr commented Jul 10, 2023

Any date to release this one? It is really required. Thanks!

@lrljoe
Copy link
Collaborator Author

lrljoe commented Jul 10, 2023

I'm just waiting on @rappasoft to give it a quick skim, but it'll be in the next release, most likely next weekend.

@rappasoft
Copy link
Owner

Looks good to me!

@lrljoe
Copy link
Collaborator Author

lrljoe commented Jul 10, 2023

Thanks @rappasoft Will merge into develop later today.

@lrljoe lrljoe merged commit 7bae3ed into rappasoft:develop Jul 10, 2023
@lrljoe
Copy link
Collaborator Author

lrljoe commented Jul 10, 2023

Now in develop - feel free to test if you are using the develop branch @rhnkyr

@lrljoe lrljoe deleted the sortableLabelFixesNew branch July 15, 2023 22:33
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.

3 participants