Fix UX regression in Dashboard Tabs Panel
ClosedPublic
Actions

Authored by valerio.bozzolan on Jun 12 2023, 14:22.

Details

Reviewers

Group Reviewers

Maniphest Tasks

T15146: Dashboards: Tab panel sometimes displays the content of two tabs
T15474: Dashboard Tabs is not open anymore as default at startup

Commits

rP659e13fa6d0e: Fix UX regression in Dashboard Tabs Panel

Summary

Before this change, it seems that the Dashboard Tabs Panel could be empty
as default. This was the regression:

cbc0e661544a31290099ab3d88481bb8d4a3003c

The variable $is_selected was defined inside a foreach loop, but that loop
was terminated when it was then read again. This redefines that variable in
the right scope.

Before	After

Closes T15474

Test Plan

see that now you have something visibile in your Dashboard Tabs Panel

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

valerio.bozzolan created this revision.Jun 12 2023, 14:22

Owners added a reviewer: O1: Blessed Committers.Jun 12 2023, 14:22

Herald added subscribers: Cigaryno, Matthew, tobiaswiese. · View Herald TranscriptJun 12 2023, 14:22

valerio.bozzolan requested review of this revision.Jun 12 2023, 14:22

Harbormaster completed remote builds in B550: Diff 950.Jun 12 2023, 14:22

put the change nearby the problem

Harbormaster completed remote builds in B551: Diff 951.Jun 12 2023, 14:26

valerio.bozzolan mentioned this in Z1: Phorge.Jun 12 2023, 14:26

valerio.bozzolan added inline comments.

src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php
114	Note this, that just lives inside the above loop. So it needs to be re-defined in the below loop.

Premising that I would like to see this comparison logic to be in a separate static method, but honestly it would just be an equality, so we can just have a green light for this copy-pasta.

valerio.bozzolan edited the summary of this revision. (Show Details)Jun 14 2023, 19:19

valerio.bozzolan added a task: T15474: Dashboard Tabs is not open anymore as default at startup.

I double-checked and I'm quite sure that this fixes the issue reported by the kind @speck

valerio.bozzolan added a task: T15146: Dashboards: Tab panel sometimes displays the content of two tabs.Jun 19 2023, 10:24

Sorry again for this regression @speck

valerio.bozzolan edited the summary of this revision. (Show Details)Jun 21 2023, 13:02

No worries, I haven’t had time to dig in. I wanted to review D25067

valerio.bozzolan mentioned this in 2023 Week 23.Jun 23 2023, 09:26

nice

This revision is now accepted and ready to land.Jun 26 2023, 23:44

Closed by commit rP659e13fa6d0e: Fix UX regression in Dashboard Tabs Panel. · Explain WhyJun 27 2023, 06:15

This revision was automatically updated to reflect the committed changes.