Merge pull request #191 from creative-commoners/pulls/4/hover-styling

ENH Improve hover active and focus state styling
silverstripe · Jan 29, 2024 · 449b376 · 449b376
2 parents 76b38e7 + 836d665
commit 449b376
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 13 deletions.
diff --git a/client/dist/js/bundle.js b/client/dist/js/bundle.js
diff --git a/client/dist/styles/bundle.css b/client/dist/styles/bundle.css
diff --git a/client/src/components/LinkField/LinkField.js b/client/src/components/LinkField/LinkField.js
@@ -143,7 +143,7 @@ const LinkField = ({
       ? i18n._t('LinkField.ARCHIVE_CONFIRM', 'Are you sure you want to archive this link?')
       : i18n._t('LinkField.DELETE_CONFIRM', 'Are you sure you want to delete this link?');
     if (!window.confirm(deleteText)) {
-      return;
+      return false;
     }
     let endpoint = joinUrlPaths(Config.getSection(section).form.linkForm.deleteUrl, linkID.toString());
     const parsedURL = url.parse(endpoint);

diff --git a/client/src/components/LinkField/LinkField.scss b/client/src/components/LinkField/LinkField.scss
@@ -20,3 +20,13 @@
 .link-field__save-record-first {
   padding-top: 7px;
 }
+
+.link-field__container .btn-link {
+  position: absolute;
+  right: 0;
+
+  &:focus,
+  &:not(:disabled):not(.disabled):active {
+    box-shadow: none;
+  }
+}
diff --git a/client/src/components/LinkPicker/LinkPicker.scss b/client/src/components/LinkPicker/LinkPicker.scss
@@ -86,7 +86,7 @@
   }
 
   &--draft::before {
-    background-color: $state-draft-bg;;
+    background-color: $state-draft-bg;
   }
 
   &--modified::before {
@@ -163,7 +163,7 @@
 .link-picker__link-detail {
   flex-grow: 1;
   width: 100%;
-  padding-left: 3.5rem;
+  padding: 0 5rem 0 3.5rem;
 }
 
 .link-picker__delete {

diff --git a/client/src/components/LinkPicker/LinkPickerMenu.js b/client/src/components/LinkPicker/LinkPickerMenu.js
@@ -17,7 +17,7 @@ const LinkPickerMenu = ({ types, onSelect }) => {
     toggle={toggle}
     className="link-picker__menu"
   >
-      <DropdownToggle className="link-picker__menu-toggle font-icon-plus-1" caret>
+      <DropdownToggle className="link-picker__menu-toggle font-icon-plus-1" caret color="secondary">
         {i18n._t('LinkField.ADD_LINK', 'Add Link')}
       </DropdownToggle>
       <DropdownMenu>

diff --git a/client/src/components/LinkPicker/LinkPickerTitle.js b/client/src/components/LinkPicker/LinkPickerTitle.js
@@ -97,10 +97,26 @@ const LinkPickerTitle = ({
           <span className="link-picker__url">{description}</span>
         </small>
       </div>
+      {(canDelete && !readonly) &&
+        // This is a <span> rather than a <Button> because we're inside a <Button> and
+        // trigger an error when you attempt to nest a <Button> inside a <Button>.
+        <span
+          role="button"
+          tabIndex="0"
+          className="link-picker__delete btn btn-link"
+          onKeyDown={(evt) => {
+            if ((evt.code === 'Enter' || evt.code === 'Space') && !loading) {
+              evt.nativeEvent.stopImmediatePropagation();
+              evt.stopPropagation();
+              onDelete(id);
+              evt.nativeEvent.preventDefault();
+              evt.preventDefault();
+            }
+          }}
+          onClick={stopPropagation(() => !loading ? onDelete(id) : null)}
+        >{deleteText}</span>
+      }
     </Button>
-    {(canDelete && !readonly && !disabled) &&
-      <Button disabled={loading} className="link-picker__delete" color="link" onClick={stopPropagation(() => onDelete(id))}>{deleteText}</Button>
-    }
   </div>
 };
 

diff --git a/client/src/components/LinkPicker/tests/LinkPickerTitle-test.js b/client/src/components/LinkPicker/tests/LinkPickerTitle-test.js
@@ -58,7 +58,7 @@ test('LinkPickerTitle delete button should fire the onDelete callback when not l
     })}
     />
   </LinkFieldContext.Provider>);
-  fireEvent.click(container.querySelector('button.link-picker__delete'));
+  fireEvent.click(container.querySelector('.link-picker__delete'));
   expect(mockOnDelete).toHaveBeenCalledTimes(1);
 });
 
@@ -71,7 +71,7 @@ test('LinkPickerTitle delete button should not fire the onDelete callback while
     })}
     />
   </LinkFieldContext.Provider>);
-  fireEvent.click(container.querySelector('button.link-picker__delete'));
+  fireEvent.click(container.querySelector('.link-picker__delete'));
   expect(mockOnDelete).toHaveBeenCalledTimes(0);
 });
 
@@ -80,7 +80,7 @@ test('LinkPickerTitle main button should fire the onClick callback when not load
   const { container } = render(<LinkFieldContext.Provider value={{ loading: false }}>
     <LinkPickerTitle {...makeProps({ onClick: mockOnClick })} />
   </LinkFieldContext.Provider>);
-  fireEvent.click(container.querySelector('button.link-picker__button'));
+  fireEvent.click(container.querySelector('.link-picker__button'));
   expect(mockOnClick).toHaveBeenCalledTimes(1);
 });
 
@@ -89,7 +89,7 @@ test('LinkPickerTitle main button should not fire the onClick callback while loa
   const { container } = render(<LinkFieldContext.Provider value={{ loading: true }}>
     <LinkPickerTitle {...makeProps({ onClick: mockOnClick })} />
   </LinkFieldContext.Provider>);
-  fireEvent.click(container.querySelector('button.link-picker__button'));
+  fireEvent.click(container.querySelector('.link-picker__button'));
   expect(mockOnClick).toHaveBeenCalledTimes(0);
 });