From a7937edac77fd48839f8e2bb649e9e5e176ca231 Mon Sep 17 00:00:00 2001
From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com>
Date: Thu, 30 Apr 2020 17:09:43 -0400
Subject: [PATCH] common: button typings improvements (#2139)

---
 js/src/common/components/Button.tsx     | 44 +++++++++++++++++--------
 js/src/common/components/LinkButton.tsx | 24 +++++++-------
 2 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/js/src/common/components/Button.tsx b/js/src/common/components/Button.tsx
index a8acf69ff..38b8e95c9 100644
--- a/js/src/common/components/Button.tsx
+++ b/js/src/common/components/Button.tsx
@@ -5,46 +5,64 @@ import extractText from '../utils/extractText';
 import LoadingIndicator from './LoadingIndicator';
 
 export interface ButtonProps extends ComponentProps {
+    /**
+     * A tooltip for the button.
+     */
     title?: string;
+
+    /**
+     * An html type attribute for the button.
+     */
     type?: string;
+
+    /**
+     * The name of the icon class. If specified, the button will be given a
+     * 'has-icon' class name.
+     */
     icon?: string;
 
+    /**
+     * Whether or not the button should be in a disabled loading state.
+     */
     loading?: boolean;
+
+    /**
+     * Whether or not the button is disabled. If truthy, the button
+     * will be given a 'disabled' class name, and any `onclick` handler will be
+     * removed.
+     */
     disabled?: boolean;
+
+    /**
+     * A callback to run when the button is clicked.
+     */
     onclick?: Function;
 }
 
 /**
  * The `Button` component defines an element which, when clicked, performs an
- * action. The button may have the following special props:
- *
- * - `icon` The name of the icon class. If specified, the button will be given a
- *   'has-icon' class name.
- * - `disabled` Whether or not the button is disabled. If truthy, the button
- *   will be given a 'disabled' class name, and any `onclick` handler will be
- *   removed.
- * - `loading` Whether or not the button should be in a disabled loading state.
- *
- * All other props will be assigned as attributes on the button element.
+ * action.
  *
  * Note that a Button has no default class names. This is because a Button can
  * be used to represent any generic clickable control, like a menu item.
  */
 export default class Button<T extends ButtonProps = ButtonProps> extends Component<T> {
     view() {
-        const { children, ...attrs } = this.props;
+        const attrs: T = { ...this.props };
+
+        const children = extract(attrs, 'children');
 
         attrs.className = attrs.className || '';
         attrs.type = attrs.type || 'button';
 
         // If a tooltip was provided for buttons without additional content, we also
         // use this tooltip as text for screen readers
-        if (attrs.title && !this.props.children) {
+        if (attrs.title && !children) {
             attrs['aria-label'] = attrs.title;
         }
 
         // If nothing else is provided, we use the textual button content as tooltip
-        if (!attrs.title && this.props.children) {
+        if (!attrs.title && children) {
             attrs.title = extractText(this.props.children);
         }
 
diff --git a/js/src/common/components/LinkButton.tsx b/js/src/common/components/LinkButton.tsx
index e220caaa9..b02da6918 100644
--- a/js/src/common/components/LinkButton.tsx
+++ b/js/src/common/components/LinkButton.tsx
@@ -1,24 +1,24 @@
 import Button, { ButtonProps } from './Button';
 
-interface LinkButtonProps extends ButtonProps {
+export interface LinkButtonProps extends ButtonProps {
+    /**
+     * Whether or not the page that this button links to is currently active.
+     */
     active?: boolean;
-    oncreate?: Function;
+
+    /**
+     * The URL to link to. If the current URL `m.route()` matches this,
+     * the `active` prop will automatically be set to true.
+     */
     href?: string;
+
+    oncreate?: Function;
 }
 
 /**
  * The `LinkButton` component defines a `Button` which links to a route.
- *
- * ### Props
- *
- * All of the props accepted by `Button`, plus:
- *
- * - `active` Whether or not the page that this button links to is currently
- *   active.
- * - `href` The URL to link to. If the current URL `m.route()` matches this,
- *   the `active` prop will automatically be set to true.
  */
-export default class LinkButton extends Button<LinkButtonProps> {
+export default class LinkButton<T extends LinkButtonProps = LinkButtonProps> extends Button<T> {
     static initProps(props: LinkButtonProps) {
         props.active = this.isActive(props);
     }