front end UI style and experience optimization#84
Conversation
Shenhan11
force-pushed
the
feature/ui-redesign
branch
from
057f677 to
61b8eb9
Compare
Shenhan11
force-pushed
the
feature/ui-redesign
branch
6 times, most recently
from
23f2610 to
b12a7c5
Compare
Shenhan11
force-pushed
the
feature/ui-redesign
branch
from
b12a7c5 to
af1d7e6
Compare
| }, | ||
| }, | ||
| ]); | ||
| const fetchTableData = async () => { |
There was a problem hiding this comment.
P1: Detail pages pass deviceId/nodeUid only after their own detail request resolves, but this table fetches only on mount and never refetches when props.filters changes. On card and node detail pages the embedded workload table can therefore stay global instead of being scoped to the current resource.
|
|
||
| const handleClick = async (params) => { | ||
| const handleClick = (params) => { | ||
| const name = params.data.name; |
There was a problem hiding this comment.
P2: The preview widget is cluster-wide, but this click path now resolves against the currently filtered tableData only. After filtering by IP/type/status, clicking a Top-5 node that is outside the filtered subset will incorrectly show "node not found". The previous implementation looked this up from the full node list, so this is a behavior regression.
| const { t, locale } = useI18n(); | ||
| const tableData = ref([]); | ||
| const tableLoading = ref(false); | ||
| const filters = reactive({ |
There was a problem hiding this comment.
P1: This page has the same regression as the card list: filters start from an empty reactive object and do not read route.query. The overview still links to /admin/vgpu/node/admin?isSchedulable=..., so node-status drill-downs and bookmarked filtered URLs silently lose their filter.
| }, | ||
| ); | ||
|
|
||
| const fetchTableData = async () => { |
There was a problem hiding this comment.
P1: Node detail renders this list before detail.uid is loaded, yet the table only fetches on mount. Because there is no watcher on props.filters, the embedded GPU list can remain global instead of refreshing to the current node once detail.uid arrives.
| const currentType = computed(() => filters.type || ''); | ||
| const tableData = ref([]); | ||
| const tableLoading = ref(false); | ||
| const filters = reactive({ |
There was a problem hiding this comment.
P1: This page now initializes local filters from a blank reactive object and never merges route.query. The dashboard still navigates here as /admin/vgpu/card/admin?type=..., so card-type drill-downs and bookmarked filtered URLs stop applying their filter after the TablePlus removal.
|
Summary of the review findings:
|
Shenhan11
force-pushed
the
feature/ui-redesign
branch
from
af1d7e6 to
1c3734c
Compare
Signed-off-by: Shenhan11 <han.shen@dynamia.ai>
Shenhan11
force-pushed
the
feature/ui-redesign
branch
from
1c3734c to
fe53a64
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: FouoF, Shenhan11 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
3 participants
No description provided.