alexbowers / nova-multiple-dashboard

[DEPRECATED] - Support for multiple custom dashboards in Laravel Nova
MIT License
34 stars 7 forks source link

allow ordering of dashboards #4

Closed jrmadsen67 closed 5 years ago

jrmadsen67 commented 5 years ago

This seemed to be the simplest way to allow users to set the order of the dashboards; I considered hijacking meta array for this, but I didn't want to interfere with how other projects might try to use that array & this keeps it "nice and tight" for our own use.

Happy to rename, tweak, etc

alexbowers commented 5 years ago

I haven't tested this yet, but can you confirm to me what happens if you have the following:

  1. No sort defined on any of the dashboards. Do they return alphabetically?
  2. No sort defined on some of the dashboards. Do they start with the ones sorted (regardless of number), and then alphabetically, or alphabetical first?
  3. Two with the same sort on them. Do they sort alphabetically, or randomly?
jrmadsen67 commented 5 years ago

No sort on any dashboard - is alphabetical ("Content", "G Content" , "Learners", "X Content")

sort on only one dashboard - is sorts default 0's, then random (perhaps a second sort that is always on label?)

"Learners" ( sort= 0), "G Content" ( sort= 0), "X Content" (sort=0), "Content"( sort= 20)

multiple non-zero sort values; seems to sort them by alphabetical order afterwards

"X Content" (sort=0), "Content"( sort= 20) , "G Content" ( sort= 0), "Learners" ( sort= 0)


->sortBy('label')->sortBy('sortby')

seems to fix the zero-sort issue in second one - push that?

"G Content" ( sort= 0), "X Content" (sort=0), "Content"( sort= 20) , "Learners" ( sort=20)

On Wed, Dec 12, 2018 at 9:24 AM Alex Bowers notifications@github.com wrote:

I haven't tested this yet, but can you confirm to me what happens if you have the following:

  1. No sort defined on any of the dashboards. Do they return alphabetically?
  2. No sort defined on some of the dashboards. Do they start with the ones sorted (regardless of number), and then alphabetically, or alphabetical first?
  3. Two with the same sort on them. Do they sort alphabetically, or randomly?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/alexbowers/nova-multiple-dashboard/pull/4#issuecomment-446414665, or mute the thread https://github.com/notifications/unsubscribe-auth/AAyKxjBEgtZPPuOhb9ow-spRKka9QHndks5u4EysgaJpZM4ZObDW .

alexbowers commented 5 years ago

So how come the order there isn't G Content Learners X Content Content for the 3rd option? Should it not be sorted by sort = 0 first and then sort = 20 and all of the sort = 0 are then done alphabetically subsequently.

jrmadsen67 commented 5 years ago

I don't follow - you mean on my fix?

That's what it does - first the two "0's", and they are sorted alpha, then the two "20's" and they are sorted alpha. They are ascending alpha

On Wed, Dec 12, 2018 at 10:07 AM Alex Bowers notifications@github.com wrote:

So how come the order there isn't X Content G Content Learners Content for the 3rd option? Should it not be sorted by sort = 0 first and then sort = 20 and all of the sort = 0 are then done alphabetically subsequently.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/alexbowers/nova-multiple-dashboard/pull/4#issuecomment-446422854, or mute the thread https://github.com/notifications/unsubscribe-auth/AAyKxglevcbjSS_zp4uIY7GkDqJ3N1Uwks5u4FbEgaJpZM4ZObDW .

alexbowers commented 5 years ago

Is there a typo in your last example then, because you have 3 set as sort =0 and 1 with sort = 20

jrmadsen67 commented 5 years ago

I see, yes there was. That is working as desired now anyway

On Wed, Dec 12, 2018 at 10:15 AM Alex Bowers notifications@github.com wrote:

Is there a typo in your last example then, because you have 3 set as sort =0 and 1 with sort = 20

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/alexbowers/nova-multiple-dashboard/pull/4#issuecomment-446424550, or mute the thread https://github.com/notifications/unsubscribe-auth/AAyKxgh8BpHHghRub84psOPOqfoDPeKiks5u4FjPgaJpZM4ZObDW .

alexbowers commented 5 years ago

ok great, ill test it tomorrow

alexbowers commented 5 years ago

Thanks, this has been merged.

alexbowers commented 5 years ago

Release 1.0.2 has also been released which contains this.