Skip to content

fix(dashboard): performance and refactoring #41065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 29, 2025

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Oct 23, 2023

Summary

⚠️ Chained on #41063

Polish a bit dashboard app:

  • Decrease duplication of dashboard v1/v2
  • Polish styles to replace classes like panel-activity--header--icon--description and very nested selectors with selectors by tags
  • Improve dragging performance:
    • Dragging dashboard was very laggy
    • Hidden icon description with .hidden-visually has transforms that makes it terrible in performance with a combination of Draggable
    • This description is not needed, icon is hidden for a11y
    • Remove the hidden description
Before After
dashboard-laggy dashboard-after

Checklist

@skjnldsv
Copy link
Member

@ShGKme ShGKme (Grigorii K. Shartsev) requested review from st3iny, susnux, JuliaKirschenheuter, skjnldsv, Pytal and szaimen 16 hours ago

Not ready for review?
image

@st3iny
Copy link
Member

st3iny commented Oct 24, 2023

@ShGKme You might have accidentally committed a dev build :D

Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works. I couldn't spot regressions.

Nice work!

@ShGKme
Copy link
Contributor Author

ShGKme commented Oct 24, 2023

Not ready for review?

@ShGKme You might have accidentally committed a dev build :D

It was in draft waiting for the base branch #41063 to be merged.

Rebased now and ready for review.

@ShGKme ShGKme force-pushed the fix/dashboard--performance-and-refactoring branch from 86f8b4f to da88d22 Compare October 24, 2023 08:54
@ShGKme ShGKme marked this pull request as ready for review October 24, 2023 08:54
@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 24, 2023
Comment on lines 17 to 37
<div v-for="panelId in layout" :key="panels[panelId].id" class="panel">
<h2 class="panel__header">
<span aria-hidden="true" class="panel__header-icon" :class="getWidgetIconClass(panels[panelId])" />
{{ getWidgetTitle(panels[panelId]) }}
</h2>
<div class="panel__content" :class="{ loading: !isApiWidgetV2(panels[panelId].id) && !panels[panelId].mounted }">
<ApiDashboardWidget v-if="isApiWidgetV2(panels[panelId].id)"
:widget="apiWidgets[panels[panelId].id]"
:data="apiWidgetItems[panels[panelId].id]"
:loading="loadingItems" />
<div v-else :ref="panels[panelId].id" :data-id="panels[panelId].id" />
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing the amount of scoping done here as well as methods used that could be computed, we would benefit from this being a chidren Vue Component directly :)

<ApiDashboardPanel v-for="panelId in layout"
	:key="panels[panelId].id"
	:panel="panels[panelId]" />

Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nice!

@ShGKme ShGKme added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 26, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
@blizzz blizzz mentioned this pull request Nov 14, 2023
@ShGKme ShGKme force-pushed the fix/dashboard--performance-and-refactoring branch from da88d22 to 770f8d1 Compare May 28, 2025 17:02
@ShGKme ShGKme marked this pull request as ready for review May 28, 2025 17:02
@ShGKme ShGKme requested review from a team as code owners May 28, 2025 17:02
@ShGKme ShGKme requested review from nfebe and skjnldsv and removed request for a team May 28, 2025 17:02
@ShGKme ShGKme added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 28, 2025
ShGKme added 2 commits May 29, 2025 09:18
- `aria-labelledby` is not needed here, it is a hidden icon
- `visually-hidden` has transformations that have huge performance
impact in combination with other transformations, for example, on
draggable

Signed-off-by: Grigorii K. Shartsev <[email protected]>
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme ShGKme force-pushed the fix/dashboard--performance-and-refactoring branch from 770f8d1 to a68cd1e Compare May 29, 2025 07:21
@ShGKme
Copy link
Contributor Author

ShGKme commented May 29, 2025

  • Rebased

@ShGKme ShGKme enabled auto-merge May 29, 2025 07:21
@ShGKme
Copy link
Contributor Author

ShGKme commented May 29, 2025

🔴 Lint php-cs is unreleated

@ShGKme ShGKme merged commit 42bfbc6 into master May 29, 2025
123 of 124 checks passed
@ShGKme ShGKme deleted the fix/dashboard--performance-and-refactoring branch May 29, 2025 07:34
@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 2, 2025

/backport 8ba8c26 to stable31

@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 2, 2025

/backport 8ba8c26 to stable30

@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 2, 2025

/backport 8ba8c26 to stable29

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants