fix(dashboards): Move generated dashboards to bottom by default #17070
Conversation
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
There was a problem hiding this comment.
I'll let team PA be the final reviewer of this. This is definitely way better than having a bunch of unwanted dashboards polluting the table, though do people even click them from the dashboards page itself at all? Why not just filter them out for just the dashboards page completely? 🤔
|
I wanted them around to let people edit things, but good point, I can link them directly to the dashboard to edit from the usage tab itself. Will do that, and remove all mention from the list. 👍 |
| (a.name ?? 'Untitled').localeCompare(b.name ?? 'Untitled') | ||
| ) | ||
| return [...Object.values(rawDashboards)] | ||
| .filter((dashboard) => !(dashboard.name ?? 'Untitled').startsWith(GENERATED_DASHBOARD_PREFIX)) |
There was a problem hiding this comment.
Wait, if the ones starting with GENERATED_DASHBOARD_PREFIX are filtered out, it doesn't make sense to sort by that prefix, right?
There was a problem hiding this comment.
Not here, but, this isn't the only place the comparison function is used, and I wanted to keep the function standard for sorting wherever you want to sort dashboards, so if, say, you're sorting raw dashboards somewhere, they're still at the end. Seemed less error prone.
Happy to remove generated prefix from the sorting function if you feel strongly about this
There was a problem hiding this comment.
Right, but nameSortedDashboards is the source for all our lists of dashboards, so if we filtered out the generated dashboards, they won't be displayed at the bottom anymore – they will be just hidden. From the description of the PR I thought that that's not the intention
There was a problem hiding this comment.
original intention yes, but based on Li's comment, am removing them completely from the list, and adding a link to the usage dashboard to get to the main db, will update description 😅
There was a problem hiding this comment.
Oh, I missed that, was still looking at the PR title 😅 In that case nameCompareFunction handling is unused in this PR, so would be better to revert that addition
There was a problem hiding this comment.
I'd argue that it at least unifies the current 3 places where we manually write the same function again, would you be fine with me removing the generated prefix from it but keeping the function, or do you want me to get rid of it completely?
There was a problem hiding this comment.
No, yeah, the function makes sense. Just the prefix handling that's not going to be used
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
|
@Twixes 0.05 didn't help, so I bumped up to 0.08 |
Problem
We generate dashboards for users for all feature flags.
This can lead to annoyances when listing dashboards, as generated ones overwhelm your own.
This PR removes them from the list, and only allows editing these dashboards via the feature flag usage tab, where they were generated.
This PR makes it such that generated ones are at the bottom by default, unless:Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
How did you test this code?