korbit-ai[bot] commented on code in PR #34596:
URL: https://github.com/apache/superset/pull/34596#discussion_r2260192708
##########
superset-frontend/src/views/CRUD/utils.tsx:
##########
@@ -367,8 +367,8 @@
display: grid;
justify-content: start;
grid-gap: ${theme.sizeUnit * 12}px ${theme.sizeUnit * 4}px;
- grid-template-columns: repeat(auto-fit, 300px);
- max-height: ${showThumbnails ? '314' : '148'}px;
+ grid-template-columns: repeat(auto-fit, 400px);
Review Comment:
### Non-responsive grid layout <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Fixed width grid columns can cause layout performance issues on mobile
devices and smaller screens due to forced horizontal scrolling.
###### Why this matters
The layout will not adapt efficiently to different screen sizes, potentially
causing jank and reflows during scrolling on mobile devices.
###### Suggested change ∙ *Feature Preview*
Use a dynamic minmax approach with clamp for better responsive behavior:
```typescript
grid-template-columns: repeat(auto-fit, minmax(min(100%, 400px), 1fr));
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f2709af0-5db0-47ac-90a2-b492e2c475ae/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f2709af0-5db0-47ac-90a2-b492e2c475ae?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f2709af0-5db0-47ac-90a2-b492e2c475ae?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f2709af0-5db0-47ac-90a2-b492e2c475ae?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f2709af0-5db0-47ac-90a2-b492e2c475ae)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:a653eb84-0dcd-4ede-a64b-8f1ff2a520b3 -->
[](a653eb84-0dcd-4ede-a64b-8f1ff2a520b3)
##########
superset-frontend/src/views/CRUD/utils.tsx:
##########
@@ -367,8 +367,8 @@ export const CardContainer = styled.div<{
display: grid;
justify-content: start;
grid-gap: ${theme.sizeUnit * 12}px ${theme.sizeUnit * 4}px;
- grid-template-columns: repeat(auto-fit, 300px);
- max-height: ${showThumbnails ? '314' : '148'}px;
+ grid-template-columns: repeat(auto-fit, 400px);
+ max-height: ${showThumbnails ? '414' : '148'}px;
Review Comment:
### Container height constraint issue <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The max-height increase doesn't account for potential grid overflow with
larger cards.
###### Why this matters
With larger card dimensions (400px width), the container's max-height may
not accommodate multiple rows of cards properly, potentially causing content to
be cut off.
###### Suggested change ∙ *Feature Preview*
Adjust the max-height to accommodate the new card dimensions or consider
making it dynamic based on the number of rows:
```typescript
max-height: ${showThumbnails ? '474' : '148'}px;
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d0bc8a79-16c1-424b-9d87-b51e41c9abc5/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d0bc8a79-16c1-424b-9d87-b51e41c9abc5?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d0bc8a79-16c1-424b-9d87-b51e41c9abc5?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d0bc8a79-16c1-424b-9d87-b51e41c9abc5?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d0bc8a79-16c1-424b-9d87-b51e41c9abc5)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:da8b9d33-1e82-41eb-9d1c-2133ef8b29fa -->
[](da8b9d33-1e82-41eb-9d1c-2133ef8b29fa)
##########
superset-frontend/src/pages/DashboardList/index.tsx:
##########
@@ -712,16 +712,16 @@ function DashboardList(props: DashboardListProps) {
});
}
- if (canCreate) {
- subMenuButtons.push({
- icon: <Icons.PlusOutlined iconSize="m" />,
- name: t('Dashboard'),
- buttonStyle: 'primary',
- onClick: () => {
- navigateTo('/dashboard/new', { assign: true });
- },
- });
- }
+ // if (canCreate) {
+ // subMenuButtons.push({
+ // icon: <Icons.PlusOutlined iconSize="m" />,
+ // name: t('Dashboard'),
+ // buttonStyle: 'primary',
+ // onClick: () => {
+ // navigateTo('/dashboard/new', { assign: true });
+ // },
+ // });
+ // }
Review Comment:
### Create Dashboard Button Disabled <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code that allows users to create new dashboards has been commented out,
removing a critical functionality that users with creation permissions should
have access to.
###### Why this matters
Users with dashboard creation permissions will no longer be able to create
new dashboards through the UI, severely limiting the application's core
functionality.
###### Suggested change ∙ *Feature Preview*
Uncomment the code block to restore the dashboard creation functionality:
```typescript
if (canCreate) {
subMenuButtons.push({
icon: <Icons.PlusOutlined iconSize="m" />,
name: t('Dashboard'),
buttonStyle: 'primary',
onClick: () => {
navigateTo('/dashboard/new', { assign: true });
},
});
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5fb6a603-540a-448d-81d2-5b7d8eed5120/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5fb6a603-540a-448d-81d2-5b7d8eed5120?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5fb6a603-540a-448d-81d2-5b7d8eed5120?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5fb6a603-540a-448d-81d2-5b7d8eed5120?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5fb6a603-540a-448d-81d2-5b7d8eed5120)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:d4ebc0d8-7d61-4246-8afe-6cc310cb8619 -->
[](d4ebc0d8-7d61-4246-8afe-6cc310cb8619)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
