Skip to content

Conversation

@elizabetdev
Copy link
Contributor

@elizabetdev elizabetdev commented Jan 11, 2023

Summary

This PR updates the confirm modal examples to reflect better writing practices and closes #6506.

It also removes the word "discard" from "Avoid these words in buttons" in: https://eui.elastic.co/#/navigation/button/guidelines.

All the discussions that led to these changes can be found in #6506.

confirm-modals-b-a@2x

QA

The confirm modals for QA can be found here: https://eui.elastic.co/pr_6519/#/layout/modal#confirm-modal

General checklist

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs and playground toggles
  • Added documentation
  • Checked Code Sandbox works for any docs examples
  • Added or updated jest and cypress tests
  • Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • Updated the Figma library counterpart
  • A changelog entry exists and is marked appropriately

@elizabetdev elizabetdev added documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) labels Jan 11, 2023
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6519/

@elizabetdev elizabetdev marked this pull request as ready for review January 12, 2023 15:34
Copy link
Contributor

@florent-leborgne florent-leborgne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update! LGTM, with a small suggestion to avoid asking a 2nd question in addition to the title.

@elizabetdev elizabetdev enabled auto-merge (squash) January 12, 2023 15:42
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6519/

@elizabetdev elizabetdev merged commit e0ac6a2 into elastic:main Jan 12, 2023
cee-chen added a commit that referenced this pull request Jan 24, 2023
* [EuiToolTip] Add `repositionOnScroll` prop (#6515)

* Add `repositionOnScroll` prop and event listener

* [Docs] Add fixed tooltip example

+ add auto-focus behavior for keyboard users

* changelog

* Updated changelog.

* 72.2.0

* Updated documentation.

* [EuiDataGrid] Fix nested interactive controls axe error (#6517)

* Fix nested interactive controls error in column visibility control

* Restore mouse experience by making switch "label" draggable

- this (almost) restores previous draggable functionality - the column name and handle are both mouse draggable, while the switch is not as it is its own interactive thing

* Fix nested interactive controls error in column sorting control

- the X button and sort button group should not be draggable as they are interactive

* Improve mouse experience by making column name draggable

- The x button and sort button group are interactive and cannot be draggable, but the column name/token can be

+ tweak padding to belong to the column name, for extra grabbable area

* [opinionated] Fix multiple CSS styles to actually work correctly

- whether or not these style changes are wanted will have to be approved by a designer

* changelog

* Improve Kibana upgrade docs (#6518)

- add table of contents

- order steps more generally in chronological order so it's easier to follow vs jumping around the page

- update recommendations and general context

* [Docs] Update `EuiConfirmModal` examples (#6519)

* [Docs] Update `EuiConfirmModal` examples

* Adding a width

* Update src-docs/src/views/modal/confirm_modal.tsx

Co-authored-by: florent-leborgne <[email protected]>

Co-authored-by: florent-leborgne <[email protected]>

* [EuiToolTip] Enforce only one visible tooltip at a time (#6520)

* [misc cleanup] Group relative imports by general concept

- keep parent services together, keep tooltip-specific imports together

* Add tooltip manager that hides all other tooltips when a new tooltip is shown

* Write Cypress E2E tests for multiple tooltip behavior

* Changelog

* Unit tests for control of internal tooltip visibility state via `ref` (#6522)

* [Docs] Fix code snippet (#6526)

* Added a11y specs for EuiNotificationEvent, EuiPageHeader, EuiPortal (#6524)

* Added a11y specs for EuiNotificationEvent, EuiPageHeader, EuiPortal

* Added a11y and keyboard specs for EuiNotificationEvent.
* Added page header a11y tests.
* Added a11y and keyboard specs for EuiPortal.

* Simplifying Portal test, adding breadcrumb and tabs to Page Header.

* Simplified two components, added a state check to EuiNotification.

* Further reducing the EuiPortal specs to test just that one component.

* update i18ntokens

* Updated changelog.

* 73.0.0

* Updated documentation.

* Check that the `upstream` remote is correctly set before proceeding in release script (#6528)

* [EuiModalHeaderTitle] Remove automatic detection of title contents in favor of always wrapping a `h1`, configurable via new `component` prop (#6530)

* Use a `component` prop for EuiModalHeaderTitle tag wrapper instead of trying to detect child types

- this is because Kibana's `<FormattedMessage>` component (which only outputs a string) is incorrectly not getting styles applied to it

* Add prop unit tests

+ convert tests to RTL while we're here

* Allow `EuiConfirmModal`s to override title component tag as well if necessary

* Changelog

* tweak changelog copy a bit more

* [Docs EuiTable] Replace all the demos using `data_store` with `faker-js` (#6521)

* Replacing `data_store` with `faker-js`

* Converted `actions.js` to `tsx`

* Converted `auto.js` to `.tsx`

* Converted `expanding_rows.js` to `*.tsx`

* Converted `footer.js` to `*.tsx`

* Converted `mobile.js` to `*.tsx`

* Converted `selection.js` to `*.tsx`

* Converted `sorting.js` to `*.tsx`

* Fix typing/`any` usages

- provide correct EuiBasicTable typescript usages/definitions

* Clean up divs and Fragments

* Simplify & clean up delete selected items code

+ remove it entirely from `footer` and `expanding_rows` examples - those demos really don't need that behavior at all

* Remove unnecessary `renderStatus` abstraction

- not being used in multiple places, so no need to DRY it out

* Fix incorrectly rendering mobile names

+ DRY out `renderStatus`
- not totally clear to me why those examples have such complex mobile examples

+ add a `mobileOptions.only` example in the `mobile` demo to make up for removed examples

* Remove unused `sortable`

- there's no sorting being passed to the table, so it does nothing

* Clean up table layout example

- remove need for randomly generated group IDs
- simplify var names
- use value passed directly from `onChange` instead of using a `.find()`
- use flex group for button toggles instead of `&emsp;`s

* Clean up emoji flag logic

- move `getEmojiFlag` util to bottom of the file - it's not relevant to anything table-related and shouldn't  be the first thing a dev has to read/parse

- remove unnecessary extra flag/countryCode() call

* Remove unnecessary data length abstraction/logic

- if `userLength` isn't being used anywhere else, just hard code it in - no need for a var

- remove unnecessary `filteredUsers` logic

+ remove unnecessary `RIGHT_ALIGNMENT` import

* Organize table/component logic by concept

- move `columns` out to static init where logical

- add headings to sort out manual pagination/sorting logic where it isn't necessarily relevant to the demo (but aids in visual output)

- add comment to `findUsers` to more clearly explain what it's doing

- remove unnecessary abstractions for longer files

* Removed `getEmojiFlag`

* Remove lodash `uniqBy` dependency

- replace one instance with a `Set` example for uniqueness

* Tweak new location column to truncate

- this column is now fairly long compared to before - let's restore the previous table visual apperances by truncating

* Standardize inconsistent link behavior

- it doesn't really make sense to have a last name be a link as opposed to a username - some demos were doing this, some weren't, so standardize the rendering

- remove actual external links to github - most of the newly random usernames aren't valid in any case

Co-authored-by: Constance Chen <[email protected]>

* Removed domain check from EuiLink (#6535)

This PR changes the behavior of the EUILink component to apply rel="noreferrer" universally for all external links, as it's considered a best practice.

Prior to this PR, there was an exception for elastic.co domains, added in #1565. As this behavior is no longer required, we're opting to remove the exception.

* [EuiBasicTable] Fix row heights jumping when actions are disabled (#6538)

* [EuiBasicTable] Fix row heights jumping when actions are disabled

* Update snapshots

* changelog

* Adding EuiModal callout for H1 rendering. (#6497)

* Adding EuiModal callout for H1 rendering.

* Updating text to be more concise and clear.

* Updating documentation for default H1 wrapper on EuiModalHeaderTitle.

* Removing example callout, added explanation to component type definition.

* Removed extra H1 tags from Docs examples.

* Removed H1 from EUIModal component and a11y specs.

* Update checklist template to recommend `@default` jsdoc usage (#6541)

* [Emotion] Convert EuiBasicTable (#6539)

* [tech debt] convert `useEuiTheme` tests to RTL `renderHook`

- which is generally a nicer API than the one I yolo'd

* [tech debt] Add more missing unit tests for `useEuiTheme`

* [tech debt] write basic unit test for `withEuiTheme`

* Add new `RenderWithEuiTheme` render prop util

* Convert `tbody` loading styles to Emotion

- I opted not to create a top-level component for this due to the very limited styles being applied, and due to HOC/theme access shenanigans

* Fix error/empty states not rendering loading styles

- by only rendering one `<tbody>`, not multiple

* Write basic `loading` test
+ switch `render` to RTL

* [extra] Massive clean up of EuiBasicTable unit tests

- switch to RTL totally (shallow was not handling the new render prop well)
- DRY out various repeated props
- stop use snapshots for every single test - use specific assertions instead. For visual rendering for various prop combos, we should use Storybook
- leave snapshots in for two specific render tests - barebones & kitchen sink props

* Delete scss files

* Add `shouldRenderCustomStyles` test

* changelog

* Add affordance for reduced motion media query

- this matches how EuiProgress behaves

+ clean up animation shorthand

* Add CSS workaround/fix for visual Safari bug

- apparently `position: relative` on the parent and not on the `tbody` was a cross-browser fix :(

* [EuiResizablePanel] Added tabindex prop to EuiResizablePanel for keyboard accessibility. (#6534)

* Added tabindex prop to EuiResizablePanel for keyboard accessibility.
* Added unit test and corrected tabIndex type.
* Renaming tabindex to tabIndex for consistency in docs.
* Renaming test description. Removing changelog.

Co-authored-by: Jason Stoltzfus <[email protected]>
Co-authored-by: Elizabet Oliveira <[email protected]>
Co-authored-by: florent-leborgne <[email protected]>
Co-authored-by: Trevor Pierce <[email protected]>
Co-authored-by: Bree Hall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dangerous confirm modal best practices

3 participants