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

New look of patch board with bound values #1684

Merged
merged 1 commit into from
Feb 14, 2019

Conversation

brusherru
Copy link
Contributor

It closes #1678

Vois-la

Copy link
Member

@knopki knopki left a comment

Choose a reason for hiding this comment

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

Awesome 🥇

Copy link
Contributor

@evgenykochetkov evgenykochetkov left a comment

Choose a reason for hiding this comment

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

Looks awesome 👍🏻

But there are a few inconsistencies left:

because pins got smaller, they look off in the inspector
screen shot 2019-02-13 at 18 10 59

comment border did not change:
screen shot 2019-02-13 at 18 13 14
(is this because of #1380?)

lines to input pins in the suggester look off:
screen shot 2019-02-13 at 18 18 22

@@ -3,6 +3,10 @@
}

#patch_bg_pattern {
g {
// Compensate bluring
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

const textVerticalOffset = PIN_RADIUS + TEXT_OFFSET_FROM_PIN_BORDER;
const isInput = direction === PIN_DIRECTION.INPUT;

const textProps = {
x: position.x,
y: position.y + textVerticalOffset * (isInput ? -1 : 1),
y: position.y - textVerticalOffset * (isInput ? -1 : 1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
y: position.y - textVerticalOffset * (isInput ? -1 : 1),
y: position.y + textVerticalOffset * (isInput ? 1 :- 1),

@evgenykochetkov
Copy link
Contributor

evgenykochetkov commented Feb 13, 2019

Also, bound values are not shown in latest the Firefox and Chromium on macOS :(
In Electron version everything looks great.

@evgenykochetkov
Copy link
Contributor

values bound to generic pins are not colored:
screen shot 2019-02-13 at 18 44 34

@brusherru brusherru force-pushed the feat-1678-bound-values-on-patch-board branch from fc13911 to 9bbccd7 Compare February 14, 2019 09:39
@brusherru
Copy link
Contributor Author

All fixed and tweaked :)

Vois-la-2

Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

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

Supa-pupa!

A flaw I noted is in the resize handle:

image

packages/xod-client/src/editor/components/PatchDocs.jsx Outdated Show resolved Hide resolved
packages/xod-client/src/project/components/PinLabel.jsx Outdated Show resolved Hide resolved
* 3○
*
* 2○
* 1○
Copy link
Member

Choose a reason for hiding this comment

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

❤️ I like ASCII pictures

@nkrkv
Copy link
Member

nkrkv commented Feb 14, 2019

I think it’s a bit better to put node labels 2px higher: 10px top margin instead of 12px

@brusherru brusherru force-pushed the feat-1678-bound-values-on-patch-board branch 3 times, most recently from 2a85486 to ee2f778 Compare February 14, 2019 13:25
@brusherru brusherru requested a review from nkrkv February 14, 2019 13:28
Copy link
Contributor

@evgenykochetkov evgenykochetkov left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

@brusherru brusherru force-pushed the feat-1678-bound-values-on-patch-board branch from ee2f778 to 3cf820c Compare February 14, 2019 15:49
Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

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

Wooohooo🤘

@brusherru brusherru merged commit c325d24 into master Feb 14, 2019
@brusherru brusherru deleted the feat-1678-bound-values-on-patch-board branch February 14, 2019 16:54
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.

Improve patch board design to show bound values
4 participants