Skip to content
This repository has been archived by the owner on Jun 29, 2023. It is now read-only.

removed text node from fitler label. #574

Merged
merged 4 commits into from
Nov 22, 2019
Merged

Conversation

pr3tori4n
Copy link
Contributor

Related Issue: #333

Summary

@pr3tori4n pr3tori4n requested a review from a team as a code owner November 20, 2019 00:18
@pr3tori4n pr3tori4n self-assigned this Nov 20, 2019
@pr3tori4n pr3tori4n added the enhancement New feature request for an existing component label Nov 20, 2019
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

There still needs to be a prop textFilterLabel for a user to be able to set this text. Otherwise, its still a hard coded value.

@pr3tori4n
Copy link
Contributor Author

@driskull There is a prop textLabel

@driskull
Copy link
Member

@pr3tori4n ok

fixme: should we update the doc description? It no longer appears there right?

/**
   * A text label that will appear next to the input field.
   */

nitpick: Could the text filter just be set by default and not have the fallback in render? This gives the user the ability to get rid of it if they really want to.

  @Prop() textLabel = Text.filterLabel;

@pr3tori4n
Copy link
Contributor Author

pr3tori4n commented Nov 21, 2019

nitpick: Could the text filter just be set by default and not have the fallback in render? This gives the user the ability to get rid of it if they really want to.

On second thought no, there needs to be something there or else this component will fail a11y.

Updated the comment.

Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

Good once comment updated on doc.

@pr3tori4n pr3tori4n merged commit d53be8d into master Nov 22, 2019
@pr3tori4n pr3tori4n deleted the hrobbins/filter-label-333 branch November 22, 2019 19:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature request for an existing component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants