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.

Sign up for GitHub

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

Jump to bottom

Add ability to send text lines via serial #1660

Merged
merged 3 commits into from
Feb 6, 2019
Merged

Add ability to send text lines via serial #1660

merged 3 commits into from
Feb 6, 2019

Conversation

evgenykochetkov
Copy link
Contributor

Closes #1622

@evgenykochetkov evgenykochetkov self-assigned this Feb 4, 2019
nkrkv
nkrkv requested changes Feb 4, 2019
View reviewed changes
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.

Now I have the Deployment pane always expanded on IDE start. I’d like to have it collapsed by default as it was previously.

packages/xod-client-electron/src/shared/events.js Outdated Show resolved Hide resolved
packages/xod-client-electron/src/upload/components/PortSelect.jsx
isDeploymentInProgress: false,
};

export default PortSelect;
Copy link
Member

Choose a reason for hiding this comment

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

👍 For extracting to a module

@nkrkv
Copy link
Member

nkrkv commented Feb 4, 2019

Also, I think a dark theme for the serial input will work nicer. A rough sketch:

image

The background color is the same used in the C++ editor (#3a3a40). The border is ~#555555.

Also, note please, that the input is not attached to the very bottom. I see a 2-3px gap there.

brusherru
brusherru requested changes Feb 4, 2019
View reviewed changes
Copy link
Contributor

@brusherru brusherru left a comment •
edited by evgenykochetkov
Loading

Choose a reason for hiding this comment

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

It works fine with the board.

But it introduced two bugs into Simulation mode:

  1. Debugger tab has no top panel with breadcrumbs and "Stop" button
  2. Watches looks inactive

Also, I've discovered one bug in the Simulation mode (closing debugger tab doesn't stop simulation) and I'll fix it very soon :)

@brusherru
Copy link
Contributor

brusherru commented Feb 4, 2019 •
edited
Loading

Also, I think it will be nice to select the baud rate in the popup...

@evgenykochetkov evgenykochetkov force-pushed the feat-send-data-to-serial branch 3 times, most recently from 1e926bd to 587bfd2 Compare February 4, 2019 11:50
@evgenykochetkov
Copy link
Contributor Author

@nkrkv @brusherru Fixed! Check again, please

nkrkv
nkrkv approved these changes Feb 4, 2019
View reviewed changes
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.

A perfectionist inside me screams

image

  • Now the input overflows the window area so that its bottom border is invisible. It would be nice if the input is placed 1px higher.
  • Also, a placeholder like “Line to send via serial” will bring a bit more clarity.

These points are little annoyances. LGTM already 👍

brusherru
brusherru approved these changes Feb 4, 2019
View reviewed changes
Copy link
Contributor

@brusherru brusherru left a comment •
edited
Loading

Choose a reason for hiding this comment

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

A few minor UX/UI issues:

  1. Make it more transparent when it's disabled. Because now it looks like something broken. Also, it will be cool to change placeholder text on something like "Connect to Serial first"
  2. Disable hover effect on disabled "send" button, please
  3. 2-3px gap is still here

In the rest, LGTM

@evgenykochetkov evgenykochetkov merged commit f5a0b4f into master Feb 6, 2019
@evgenykochetkov evgenykochetkov mentioned this pull request Feb 8, 2019
Merged
@evgenykochetkov evgenykochetkov deleted the feat-send-data-to-serial branch February 13, 2019 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brusherru brusherru brusherru approved these changes

@nkrkv nkrkv nkrkv approved these changes

Assignees

@evgenykochetkov evgenykochetkov

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@evgenykochetkov @nkrkv @brusherru