rusackas commented on a change in pull request #10298:
URL:
https://github.com/apache/incubator-superset/pull/10298#discussion_r455985272
##########
File path: superset-frontend/src/components/Button.tsx
##########
@@ -17,41 +17,70 @@
* under the License.
*/
import React from 'react';
-import PropTypes from 'prop-types';
import { kebabCase } from 'lodash';
import {
Button as BootstrapButton,
Tooltip,
OverlayTrigger,
} from 'react-bootstrap';
+import styled from '@superset-ui/style';
-const propTypes = {
- children: PropTypes.node,
- className: PropTypes.string,
- tooltip: PropTypes.node,
- placement: PropTypes.string,
- onClick: PropTypes.func,
- disabled: PropTypes.bool,
- bsSize: PropTypes.string,
- bsStyle: PropTypes.string,
- btnStyles: PropTypes.string,
-};
-const defaultProps = {
- bsSize: 'sm',
- placement: 'top',
-};
+export type OnClickHandler = React.MouseEventHandler<BootstrapButton>;
+
+export interface ButtonProps {
+ className?: string;
+ tooltip?: string;
+ placement?: string;
+ onClick?: OnClickHandler;
+ disabled?: boolean;
+ bsStyle?: string;
+ btnStyles?: string;
+ bsSize?: BootstrapButton.ButtonProps['bsSize'];
+ style?: BootstrapButton.ButtonProps['style'];
+}
const BUTTON_WRAPPER_STYLE = { display: 'inline-block', cursor: 'not-allowed'
};
-export default function Button(props) {
- const buttonProps = { ...props };
+const SupersetButton = styled(BootstrapButton)`
+ &.supersetButton {
Review comment:
I like that idea... then ALL the buttons usage can be migrated to
import/use the new Button component, instead of direct (React-)Bootstrap use,
and they'll be easy to find/replace as we go forward with SIP-34.
##########
File path: superset-frontend/src/components/ListView/ListView.tsx
##########
@@ -155,6 +205,37 @@ const ListView: FunctionComponent<Props> = ({
)}
</div>
<div className="body">
+ {bulkSelectEnabled && (
+ <BulkSelectWrapper bsStyle="info" onDismiss={disableBulkSelect}>
+ <div className="selectedCopy">
+ {renderBulkSelectCopy(selectedFlatRows)}
+ </div>
+ {Boolean(selectedFlatRows.length) && (
+ <>
+ <span
+ role="button"
+ tabIndex={0}
+ className="deselect-all"
+ onClick={() => toggleAllRowsSelected(false)}
+ >
+ {t('Deselect All')}
+ </span>
+ <div className="divider" />
+ {bulkActions.map(action => (
+ <Button
Review comment:
Probably debatable, but my gut tells me that one component serving both
makes sense, perhaps with some props to drive it as you've suggested. Then all
the code is in one spot, and easier to maintain/migrate/style, especially
if/when we shift into AntD.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
