Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiModal] Source order adjusted to annouce modal title before close #8233

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Cedgo1997
Copy link

@Cedgo1997 Cedgo1997 commented Dec 13, 2024

Summary

This PR introduces a workaround to solve the issue #7222, the following updates to the EuiModal and EuiModalHeader components, in addition to adjustments in the tests and styles moved based on the changed components.

  1. EuiModal Enhancements:

    • Added a mechanism to pass onClose down to children that accept it.
    • Refactored children processing to enhance each child with the onClose prop if applicable.
  2. EuiModalHeader Enhancements:

    • Added a onClose prop to support closing the modal directly from the header.
    • Introduced a close button (EuiButtonIcon) with proper accessibility (aria-label), now this close button is under the title, so it'll be annouce after.

Changes in modal.tsx

  • Type Addition: Introduced ChildWithOnClose type to ensure children accepting onClose are type-safe.
  • Enhanced Children: Utilized React.Children.map and React.cloneElement to inject onClose prop into child components.
  • Removed Close Icon: Responsibility for the close button is shifted to EuiModalHeader.

Changes in modal_header.tsx

  • Close Button: Added a close button using EuiButtonIcon with styles and localization via EuiI18n.
  • onClose Prop: Introduced onClose prop to facilitate parent-driven close behavior.

Detailed Changes

modal.tsx

const enhancedChildren = React.Children.map(children, (child) => {
  if (React.isValidElement(child)) {
    return React.cloneElement(child as ChildWithOnClose, { onClose });
  }
  return child;
});
  • This ensures that any child capable of handling onClose will receive it automatically, making the component more flexible.

modal_header.tsx

<EuiI18n
  token="euiModalHeader.closeModal"
  default="Closes this modal window"
>
  {(closeModal: string) => (
    <EuiButtonIcon
      iconType="cross"
      css={cssCloseIconStyles}
      className="euiModal__closeIcon"
      color="text"
      onClick={onClose}
      aria-label={closeModal}
    />
  )}
</EuiI18n>
  • modal.tsx and modal_header.tsx styles were affected only to match reallocated code.

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist
    • If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)

@Cedgo1997 Cedgo1997 requested a review from a team as a code owner December 13, 2024 05:47
Copy link

👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant