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

feat(input): display type icons #713

Merged
merged 6 commits into from
Sep 29, 2023
Merged

Conversation

AykutSarac
Copy link
Member

@AykutSarac AykutSarac commented Sep 26, 2023

I decided that'd be a good idea to display icons depending on the type of the input.

icon attribute overrides the default icon.

image
<bl-input type="search" />
<bl-input type="date" />
<bl-input type="date" icon="academy" />
<bl-input type="time" />

Additionally, this PR adds input type of search, it could be good idea to review #712 first.

@muratcorlu muratcorlu requested a review from a team September 26, 2023 09:09
@muratcorlu
Copy link
Contributor

I think it will be nice to merge #712 first, then add default icons with this PR

@AykutSarac
Copy link
Member Author

I think it will be nice to merge #712 first, then add default icons with this PR

Actually #712 has no crucial relevance with this PR so this could be reviewed separately.

@ogunb
Copy link
Contributor

ogunb commented Sep 28, 2023

Should we also update the clear button for type search with this PR? Or should there be another issue for it?

@muratcorlu
Copy link
Contributor

Should we also update the clear button for type search with this PR? Or should there be another issue for it?

I think "clear button for input" is another story. Since some inputs (like search) already puts a native clear button in some browsers, but some others don't. Probably we'll need to make a generic decision that covers all types of inputs. It would be better to consider it separately.

Copy link
Contributor

@muratcorlu muratcorlu left a comment

Choose a reason for hiding this comment

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

It looks good to me.

Copy link
Contributor

@muratcorlu muratcorlu left a comment

Choose a reason for hiding this comment

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

Unit and UI Test would be great.

src/components/input/bl-input.ts Show resolved Hide resolved
Copy link
Contributor

@muratcorlu muratcorlu left a comment

Choose a reason for hiding this comment

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

Good job! 🚀

@AykutSarac AykutSarac merged commit b7d9faf into next Sep 29, 2023
7 checks passed
@AykutSarac AykutSarac deleted the feat/display-input-type-icons branch September 29, 2023 08:02
@github-actions
Copy link

🎉 This PR is included in version 2.3.0-beta.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

@AykutSarac AykutSarac mentioned this pull request Oct 12, 2023
@github-actions
Copy link

🎉 This PR is included in version 2.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

5 participants