-
Notifications
You must be signed in to change notification settings - Fork 179
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
Allows edit text as soon as user type while selecting a text element #1810
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1810 +/- ##
============================================
- Coverage 65.93% 65.92% -0.01%
Complexity 304 304
============================================
Files 635 635
Lines 10342 10340 -2
============================================
- Hits 6819 6817 -2
Misses 3523 3523
|
Size Change: -16 B (0%) Total Size: 802 kB
ℹ️ View Unchanged
|
* master: (77 commits) Fix issue templates headers Bump karma from 5.0.5 to 5.0.8 (#1851) add allow-empty-input in linter so that changes in storybook js don't fail in pre-commit (#1850) add FlagsProvider to storybook simplify jasmine eslint rules Bump @testing-library/dom from 7.5.6 to 7.5.7 (#1829) Bump react-moveable from 0.20.6 to 0.20.7 (#1827) Bump puppeteer from 3.0.4 to 3.1.0 (#1826) review fixes Karma: puppeeter support and native events ignore karma tests from coverage review fixes Update karma.config.js Update .eslintrc Incrase timeout for visual regression test (#1785) Dashboard: Adds Tooltips. Borders around Preview Cards. (#1800) Update issue & PR templates (#1824) Add docs about Stories Labs env (#1815) Bump @wordpress/e2e-test-utils from 4.6.0 to 4.7.0 (#1790) Add a basic database upgrader. (#1751) ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works great. And this also fixes #1608 btw!
* master: (101 commits) Update list of Google Fonts Fix google fonts build script Use data arg for force=true instead of as a literal string Added call to delete API for media in apiProvider Fix using incorrect input for adding color preset. (#1933) only use shapes and background colors for dummy story data to avoid local host image issues Bump percy from 0.26.6 to 0.26.7 (#1937) Renamed animation components for better clarity Add custom calendar implementation (#1743) Fixed lint errors Added tests to AMPKeyframes Bump uuid from 8.0.0 to 8.1.0 (#1909) Bump @wordpress/icons from 1.4.0 to 2.0.0 (#1928) Bring back eslint-plugin-react-hooks and fix issues (#1589) Use use-context-selector versions of createContext, useContext Add useContextSelectorShallow hook Moved SimpleAnimation to root of parts Dashboard: Converted BlinkOn animation to new standard splitting out storiesView and emptyView from content. Adding tests in content/test and storybook examples in content/ . Force stylelint to 13.3.3, because 13.5.0 was failing ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we have the new integration test setup, could we add a test for this too? Both entering edit mode by clicking, pressing enter and typing a key? And of course validate selection and text field contents afterwards?
* master: (133 commits) Update build script Update release assets workflow Bump @wordpress/data from 4.18.0 to 4.19.0 (#2114) Bump @wordpress/i18n from 3.12.0 to 3.13.0 (#2110) Bump @wordpress/e2e-test-utils from 4.7.0 to 4.8.0 (#2116) Bump @storybook/addon-actions from 6.0.0-beta.15 to 6.0.0-beta.17 (#2118) Bump lint-staged from 10.2.6 to 10.2.7 (#2113) Update eslint-plugin-react-hooks Unlock stylelint version (#1913) Bump @storybook/client-api from 6.0.0-beta.15 to 6.0.0-beta.17 (#2063) Bump @testing-library/jest-dom from 5.8.0 to 5.9.0 (#2111) Bump @storybook/addon-viewport from 6.0.0-beta.15 to 6.0.0-beta.17 (#2064) Bump @storybook/addon-knobs from 6.0.0-beta.15 to 6.0.0-beta.17 (#2065) Bump @storybook/addon-a11y from 6.0.0-beta.15 to 6.0.0-beta.17 (#2062) Bump @storybook/addon-backgrounds from 6.0.0-beta.15 to 6.0.0-beta.17 (#2061) Bump @storybook/addon-storysource from 6.0.0-beta.15 to 6.0.0-beta.17 (#2066) Bump karma-jasmine from 3.2.0 to 3.3.1 (#2102) Bump @storybook/react from 6.0.0-beta.15 to 6.0.0-beta.17 (#2068) Add storybook story for failed preview dialog (#2080) Bump @testing-library/dom from 7.5.9 to 7.6.0 (#2103) ...
* master: Karma: improve and document the debug mode Fix eslint on master reset added & tests updated update keyframes to supported format polyfill working on safari storybook wip, making sure amp anims not broken on safari WIP animations working on safari polyfill web animations api added tests & updated story to utilize new methods unifying aggregate anim methods
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
@googlebot I consent. |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are good, progressing to QA
@obetomuniz I found 3 issues (2 in gif), should we handle them here?
|
@obetomuniz, are you following up on this after QA failed? |
Summary
This PR fixes text element edit mode to allows insert content when user select text element and press some character
Relevant Technical Choices
After some debug work, I noticed that we have some similar expected behavior on the already working implementation of Enter action, so I just improve the method to allow both scenarios.
Solving #830
Fixes #1608