Skip to content

Add ability to sort widgets#745

Merged
danharrin merged 6 commits intofilamentphp:1.xfrom
underdpt:add-sort-widgets
Dec 2, 2021
Merged

Add ability to sort widgets#745
danharrin merged 6 commits intofilamentphp:1.xfrom
underdpt:add-sort-widgets

Conversation

@underdpt
Copy link
Copy Markdown
Contributor

@underdpt underdpt commented Dec 1, 2021

Hello,

This PR adds the ability to sort widgets, similar how you can sort the right menu options.

It's backwards compatible, so it won't break or change ordering of existing widgets if no order is set.

@danharrin
Copy link
Copy Markdown
Member

Hey, please move the sorting logic to FilamentManager so that plugin-registered widgets are also sorted.

Also, add the $sort attribute to the base Widget class, not the stub.

@danharrin danharrin added enhancement New feature or request pending changes labels Dec 1, 2021
@underdpt
Copy link
Copy Markdown
Contributor Author

underdpt commented Dec 2, 2021

Hello,

I've just taken a look into this, and I'm not sure which way is preferable, on FilamentManager, having in mind that

  • $widgets is a public property, so it should be ordered by default in case someone is using it directly
  • Being $widgets a public property, it can be modified by the developer, so the order would be lost

So the two options are:

  • Order $widgets when calling registerWidget(). This would be less performant, and would ignore those two points before
  • Order the widgets when calling getWidgets(). This is only called one time, so it should be more perfomant. The widgets would be ordered also in case the developer tinkers with $widgets property, but not if he uses that property directly.

I think the second one is better, what do you think?

@danharrin
Copy link
Copy Markdown
Member

Hi,

Yeah, exactly. I was hinting at using getWidgets() to process sorting.

@underdpt
Copy link
Copy Markdown
Contributor Author

underdpt commented Dec 2, 2021

Done.

I'm using a intermediate collection because it will allow for cleaner code when adding #746 to the same method.

@danharrin danharrin merged commit 2b9d532 into filamentphp:1.x Dec 2, 2021
@danharrin
Copy link
Copy Markdown
Member

Thanks!

@underdpt underdpt deleted the add-sort-widgets branch December 2, 2021 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants