EnxDev commented on code in PR #34067:
URL: https://github.com/apache/superset/pull/34067#discussion_r2210786209
##########
superset-frontend/src/pages/AnnotationLayerList/index.tsx:
##########
@@ -288,10 +284,10 @@ function AnnotationLayersList({
image: 'filter-results.svg',
buttonAction: () => handleAnnotationLayerEdit(null),
buttonText: (
- <>
+ <span className="ant-btn-icon">
Review Comment:
I believe the `EmptyState ` component could benefit from a refactor.
I've noticed that in many cases, a `<span>` with role="button" is passed
into the description just to render a button. This doesn't follow our best
practices and should be avoided.
In general, the choice between passing a `Button `component directly versus
using individual props depends on the level of modularity we want.
One possible approach could be to replace the current `button`
implementation with a prop like `actions`, which would render a dedicated
`Actions section` and allow us to insert any content we need.
Alternatively, we could keep the current structure and simply add props for
the icon and its position (e.g., start or end). However, my concern is that we
might end up introducing too many props just to replicate the same flexibility
we’d get by passing a `Button` component directly.
Whether we opt for flexibility or constraints really depends on `our needs`.
Personally, I find that passing a complete `Button` component is more
readable and powerful, since it gives access to all the available props.
On the other hand, if our use case is relatively static, a controlled,
prop-based would allow us to apply centralized changes without having to modify
each usage individually.
That said, I believe the `description` section should definitely remain
flexible, as we already have use cases that require custom content.
The button section, however, could stay more constrained; though I would
lean towards introducing an `actions` prop to allow content to be passed more
explicitly.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
