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

Document and rename showSuggestionsOverride to something more sensible #16497

Merged
merged 2 commits into from Jul 11, 2019

Conversation

@talldan
Copy link
Contributor

commented Jul 10, 2019

Description

I added a small contribution to #10128 to fix a problem when hiding the URLInput. It was pointed out in review comments that the name of the prop could be improved, and documentation should be added.

Now that #10128 has been merged, here's the follow up addressing those comments.

How has this been tested?

  1. Add a button block
  2. Type in some text in the url input field that triggers the autocomplete suggestions to display
  3. Press tab to tab out of the field

Expected behaviour

The input is hidden due to the button block no longer being selected. The autocomplete suggestions are also hidden.

Screenshots

Types of changes

Code quality improvement (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

When hiding the URLInput using CSS (as is sometimes done for accessibility purposes), the suggestions can still be displayed. This is because they're rendered in a popover in a different part of the DOM, so any styles applied to the URLInput's container won't affect the popover.

This prop allows the suggestions list to be progrmatically not rendered by passing a boolean—it can be `true` to make sure suggestions aren't rendered, or `false`/`undefined` to fall back to the default behaviour of showing suggestions when matching autocompletion items are found.

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta Jul 10, 2019

Member

progrmatically -> programmatically

@@ -223,9 +223,9 @@ class URLInput extends Component {
this.inputRef.current.focus();
}

static getDerivedStateFromProps( { showSuggestionsOverride }, { showSuggestions } ) {

This comment has been minimized.

Copy link
@jorgefilipecosta

jorgefilipecosta Jul 10, 2019

Member

Aren't we breaking back compatibility with current usages of showSuggestionsOverride? Should we keep supporting the existing prop but write a deprecated warning when it is used?

This comment has been minimized.

Copy link
@Soean

Soean Jul 10, 2019

Member

It was added in #10128 (merged 16 hours ago) so I think we don't break anything.

@noisysocks
Copy link
Member

left a comment

Thanks! 👍

@noisysocks noisysocks merged commit 4c55270 into master Jul 11, 2019

1 of 2 checks passed

Filter merged Filter merged
Details
Travis CI - Pull Request Build Passed
Details

@noisysocks noisysocks deleted the update/url-input-suggestions-override branch Jul 11, 2019

Tug added a commit that referenced this pull request Jul 12, 2019

Document and rename showSuggestionsOverride to something more sensible (
#16497)

* Document and rename showSuggestionsOverride to something more sensible

* s/progrmatically/programmatically/

jg314 added a commit to jg314/gutenberg that referenced this pull request Jul 19, 2019

Document and rename showSuggestionsOverride to something more sensible (
WordPress#16497)

* Document and rename showSuggestionsOverride to something more sensible

* s/progrmatically/programmatically/

@youknowriad youknowriad added this to the Gutenberg 6.2 milestone Jul 26, 2019

sbardian added a commit to sbardian/gutenberg that referenced this pull request Jul 29, 2019

Document and rename showSuggestionsOverride to something more sensible (
WordPress#16497)

* Document and rename showSuggestionsOverride to something more sensible

* s/progrmatically/programmatically/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.