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

style: update multiple choice input node style #1954

Merged
merged 7 commits into from
Feb 13, 2020
Merged

Conversation

alanlong9278
Copy link
Contributor

@alanlong9278 alanlong9278 commented Feb 5, 2020

Description

UI about multiple choice input align with design.

Task Item

closes #1907

Screenshots

image

image

a-b-r-o-w-n
a-b-r-o-w-n previously approved these changes Feb 5, 2020
@a-b-r-o-w-n a-b-r-o-w-n changed the title chore: Ui about multiple choice input style: update choice input node style Feb 5, 2020
@cwhitten
Copy link
Member

cwhitten commented Feb 5, 2020

Hi @alanlong9278

I believe we need to add a border radius to the think blue border on the top node edges. cc @DesignPolice

@DesignPolice
Copy link

DesignPolice commented Feb 6, 2020

Hey Alan,

not sure why I didn't get an email on this - I need to check my email settings. Sorry to not get back to you sooner.

I have a couple of current issues that also have I have noticed a similar treatment, I would love to just make a couple of tweaks.

The top corners on the nodes have a corner radius of 2px - the hoover outline should have that as well.

I have mocked up some screen shots on these two.

ChoiceInput Node Edit #1905 - these will need changes on other nodes

Hoover State on Nodes #1904 https://github.com/microsoft/BotFramework-Composer/issues/1904

but also attached here.

reach out if that you would like more clarity... happy to help out. Thank you!
M

screen grab

@@ -21,7 +21,7 @@ export const ChoiceInputChoices = ({ choices }) => {
}

return (
<div data-testid="ChoiceInput" css={{ padding: '0 0 8px 45px' }}>
<div data-testid="ChoiceInput" css={{ padding: '0 0 16px 29px' }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

can this value be calculated from some constants?

Choose a reason for hiding this comment

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

@yeze322 I'm not sure how to answer that... the goal is that ideally things are all multiples of 4.

I'm not a math guy, but happy to try and help if there is some sort of ratio you want to make that seems like a good idea... or if switching to SVG files makes life easier for you?

Copy link
Contributor

Choose a reason for hiding this comment

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

@DesignPolice Thanks Marc! I think your design has already covered those pixel numbers. Programmatically, we need some code optimization to align to your design.

@yeze322 yeze322 mentioned this pull request Feb 6, 2020
@alanlong9278 alanlong9278 changed the title style: update choice input node style style: update multiple choice input node style Feb 7, 2020
@github-actions
Copy link

Coverage Status

Coverage increased (+0.01%) to 42.28% when pulling 9e6c2cb on julong/choiceinput into 2268545 on master.

@cwhitten cwhitten merged commit 65f959b into master Feb 13, 2020
@cwhitten cwhitten deleted the julong/choiceinput branch February 13, 2020 00:04
@a-b-r-o-w-n a-b-r-o-w-n mentioned this pull request Feb 21, 2020
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.

5 participants