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

[v4] [core] feat: update input borders and separate buttons #5055

Merged
merged 7 commits into from
Dec 2, 2021

Conversation

johnly13
Copy link
Contributor

@johnly13 johnly13 commented Dec 1, 2021

Fixes #4989

Changes proposed in this pull request:

  • Update default borders
  • Detach buttons for control groups with buttons (e.g. NumericInput)
  • Inset border focus states

Reviewers should focus on:

Screenshot

@@ -41,15 +41,6 @@ $dark-control-background-color-active: linear-gradient(0deg, rgba($white, 0.1),
$input-placeholder-color: $gray1;
$dark-input-placeholder-color: $gray4;

// Input variables and overrides
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved all of these overrides + styles to components/forms/*

@@ -117,18 +117,29 @@ $pt-transition-duration: 100ms !default;

// Light theme styles

$pt-input-box-shadow: inset border-shadow(0.15),
inset 0 1px 1px rgba($black, $pt-drop-shadow-opacity) !default;
$pt-input-box-shadow: inset 0px 1px 1px rgba(17, 20, 24, 0.5),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated default borders for light theme
Screen Shot 2021-12-01 at 16 36 10

Copy link
Contributor

Choose a reason for hiding this comment

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

I think these should all use $black instead of 17, 20, 24...

Suggested change
$pt-input-box-shadow: inset 0px 1px 1px rgba(17, 20, 24, 0.5),
$pt-input-box-shadow: inset 0px 1px 1px rgba($black, 0.5),


$pt-dialog-box-shadow: $pt-elevation-shadow-4 !default;
$pt-popover-box-shadow: $pt-elevation-shadow-3 !default;
$pt-tooltip-box-shadow: $pt-popover-box-shadow !default;

// Dark theme styles

$pt-dark-input-box-shadow: inset border-shadow(0.3),
inset 0 1px 1px rgba($black, $pt-dark-drop-shadow-opacity) !default;
$pt-dark-input-box-shadow: inset 0 1px 0 0 rgba(17, 20, 24, 0.6),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated default borders for dark theme
Screen Shot 2021-12-01 at 16 36 40

@blueprint-bot
Copy link

Add gap between control group components

Previews: documentation | landing | table | modern colors demo

@@ -59,8 +59,8 @@ $control-group-stack: (
@function input-transition-shadow($color: $input-shadow-color-focus, $focus: false) {
@if $focus {
@return
border-shadow(1, $color, 1px),
border-shadow(0.3, $color, 3px);
inset border-shadow(1, $color, 1px),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

add inset focus border for light theme and reduce outer border to 2px
Screen Shot 2021-12-01 at 16 37 54

border-shadow(1, $color, 1px),
border-shadow(1, $color, 1px), // duplicating to minimize browser antialiasing in dark
border-shadow(0.3, $color, 3px);
inset border-shadow(1, $color, 1px),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

add inset focus border for dark theme and reduce outer border to 2px
Screen Shot 2021-12-01 at 16 38 17

}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed all logic for removing borders between consecutive components

}
}

// Add border radius inheritance to support components wrapped in a popover
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed all logic to unround border-radius for consecutive components

> *:not(.#{$ns}-divider) {
margin-right: -$button-border-width;
> :not(:last-child) {
margin-right: 2px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

detach buttons from input
Screen Shot 2021-12-01 at 16 39 46

with focus:
Screen Shot 2021-12-01 at 16 39 58

@@ -233,6 +233,10 @@ $input-button-height-small: $pt-button-height-smaller !default;
&.#{$ns}-intent-#{$intent} {
.#{$ns}-input {
@include pt-input-intent($color);

.#{$ns}-dark & {
@include pt-dark-input-intent(map-get($pt-dark-input-intent-box-shadow-colors, $intent));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

use of $color4 for dark mode intent input within input group moved to here
Screen Shot 2021-12-01 at 16 41 41

@@ -48,7 +48,7 @@ Styleguide input
@include pt-input-intent($color);

.#{$ns}-dark & {
@include pt-dark-input-intent($color);
@include pt-dark-input-intent(map-get($pt-dark-input-intent-box-shadow-colors, $intent));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as above but for non-input group dark inputs

@@ -11,27 +14,6 @@
min-height: 0;
padding: 0;
width: $pt-button-height;

&:first-child {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed all logic to unround border-radius for consecutive components in numeric input

@blueprint-bot
Copy link

Remove redundant margin-right in numeric-input

Previews: documentation | landing | table | modern colors demo

@blueprint-bot
Copy link

Remove gap between numeric input buttons

Previews: documentation | landing | table | modern colors demo

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

the focus style changes generally look ok... for example here's a danger intent numeric input:
2021-12-02 10 57 35

but there seems to be a problem with tag input styles, this doesn't seem right:
2021-12-02 10 57 45

@@ -117,18 +117,29 @@ $pt-transition-duration: 100ms !default;

// Light theme styles

$pt-input-box-shadow: inset border-shadow(0.15),
inset 0 1px 1px rgba($black, $pt-drop-shadow-opacity) !default;
$pt-input-box-shadow: inset 0px 1px 1px rgba(17, 20, 24, 0.5),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these should all use $black instead of 17, 20, 24...

Suggested change
$pt-input-box-shadow: inset 0px 1px 1px rgba(17, 20, 24, 0.5),
$pt-input-box-shadow: inset 0px 1px 1px rgba($black, 0.5),

@blueprint-bot
Copy link

Fix lint and use instead of rgb values

Previews: documentation | landing | table | modern colors demo

@@ -114,7 +114,7 @@ $tag-input-icon-padding-large: ($pt-input-height-large - $pt-icon-size-large) /
background-color: $input-background-color;
box-shadow: input-transition-shadow($input-shadow-color-focus, true), $input-box-shadow-focus;

@each $intent, $color in $pt-intent-text-colors {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixes the colors for tag input active borders
Screen Shot 2021-12-02 at 16 20 41

@johnly13
Copy link
Contributor Author

johnly13 commented Dec 2, 2021

added a tag input example to the demo app and updated the colors for tag input active borders

@blueprint-bot
Copy link

Add TagInputExample and fix TagInput active borders

Previews: documentation | landing | table | modern colors demo

@blueprint-bot
Copy link

Update time picker intent focus colors

Previews: documentation | landing | table | modern colors demo

@adidahiya adidahiya changed the title [v4] [core] Update Input borders and separate buttons [v4] [core] feat: update input borders and separate buttons Dec 2, 2021
@adidahiya adidahiya merged commit ec0b016 into next Dec 2, 2021
@adidahiya adidahiya deleted the jl/v4-input-revisions branch December 2, 2021 22:15
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.

3 participants