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

[TagInput] leftIconName + rightElement #1420

Merged
merged 9 commits into from
Aug 10, 2017
Merged

Conversation

giladgray
Copy link
Contributor

@giladgray giladgray commented Aug 8, 2017

Addresses #1412

Changes proposed in this pull request:

  • leftIconName renders an icon as first child
  • rightElement appears as last child
  • just like InputGroup

screen shot 2017-08-07 at 8 47 27 pm

@blueprint-bot
Copy link

rightElement appears as last child

Preview: documentation
Coverage: core | datetime

@cmslewis
Copy link
Contributor

cmslewis commented Aug 8, 2017

@giladgray can we also add support for pt-round in this PR, while we're on the topic of TagInput aesthetic extensions? That would have been the final necessary style piece for our side project last week.

@cmslewis
Copy link
Contributor

cmslewis commented Aug 8, 2017

FYI Clear button doesn't look centered in the button in your screenshot:
image

}
const iconClass = classNames(
Classes.TAG_INPUT_ICON,
useLarge ? CoreClasses.ICON_LARGE : CoreClasses.ICON_STANDARD ,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: no space before comma

@blueprint-bot
Copy link

blueprint-bot commented Aug 9, 2017

ugh remove space

Preview: documentation
Coverage: core | datetime

@giladgray
Copy link
Contributor Author

addressed all the issues: button centering and padding and whitespace.

@cmslewis i poked around .pt-round support and it's not so easy. i think we should instead try to merge the .pt-input-group and .pt-tag-input styles and ensure that they both support the same things, forever.

@@ -9,39 +9,56 @@
@import "~@blueprintjs/core/src/components/forms/common";
@import "~@blueprintjs/core/src/components/tag/common";

$tag-input-padding: ($pt-input-height - $tag-height) / 2;
$ti-padding: ($pt-input-height - $tag-height) / 2;
$ti-right-padding: ($pt-input-height - $pt-button-height-small) / 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

So much nicer with this added 👏 Perfectly squared off. $tag-input- ain't that long is it?

How about -right-padding -> -padding-right to match the property name?

@blueprint-bot
Copy link

scss variable name

Preview: documentation
Coverage: core | datetime

@giladgray
Copy link
Contributor Author

giladgray commented Aug 10, 2017

👀 on e042aa8 would be appreciated, to make sure nothing is weird

update: removed that commit, moving to separate PR. my bad ✨

@giladgray giladgray force-pushed the gg/taginput-left-right branch from 61ba3c0 to dee48b0 Compare August 10, 2017 02:05
@blueprint-bot
Copy link

fix test

Preview: documentation
Coverage: core | datetime

@giladgray giladgray merged commit bb5cf6b into master Aug 10, 2017
@giladgray giladgray deleted the gg/taginput-left-right branch August 10, 2017 02:26
@llorca llorca mentioned this pull request Aug 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants