Add tabtest viewer to IDE #1585

evgenykochetkov · 2018-12-07T11:47:14Z

Closes #1540

nkrkv · 2018-12-12T09:14:31Z

Looks nice, but I'm experiencing some strange behavior.

Steps to reproduce

Drill down to xod/math/abs tabtests
Double click the empty cell to the right of 0 on the second line to invoke the input

Expected behavior

No jumps/resizes are done, the content starts editing inline literally

Actual behavior

The cell becomes ~50px wider
The cell becomes ~4px taller

Steps to reproduce

Drill down to xod/math/abs tabtests
Span the selection to the two cells with 0's on the second row
Press Ctrl+X

Expected behavior

The contents are moved to the clipboard
The cells are erased (or will erase on paste)

Actual behavior

The whole table is replaced with

IN	OUT
"some input"	"expected output"

Ctrl+Z does not work to undo the mess

nkrkv

Scrollbars are missing if the grid overflows the work area 😢

nkrkv · 2018-12-12T09:28:16Z

packages/xod-project/src/flatten.js

@@ -85,7 +85,7 @@ const terminalOriginalDirectionLens = R.lensProp('originalDirection');
 const isLeafPatchWithImplsOrTerminal = def(
  'isLeafPatchWithImplsOrTerminal :: Patch -> Boolean',
  R.anyPass([
-    Patch.hasImpl,
+    R.both(Patch.isPatchNotImplementedInXod, Patch.hasImpl),


This check looks strange: if an apple is green and greenish.

Why can’t we have a single function to check for the implementation presence? That is, we should kill either Patch.isPatchNotImplementedInXod or Patch.hasImpl

packages/xod-client/src/editor/containers/Editor.jsx

packages/xod-client/src/project/reducer.js

packages/xod-project/built-in-patches.xodball

nkrkv · 2018-12-12T09:43:33Z

packages/xod-project/src/attachment.js

+  'createAttachmentManagedByMarker :: PatchPath -> String -> Attachment',
+  (markerName, content) =>
+    createAttachment(
+      // TODO: what about unknown filenames?


I think it’s a bug. Pick from MANAGED_ATTACHMENT_FILENAMES with Maybe, explode with assertion

nkrkv · 2018-12-12T09:44:11Z

packages/xod-project/src/flatten.js

@@ -795,6 +795,7 @@ export const extractPatches = R.curry(
 const convertPatch = def(
  'convertPatch :: Project -> [Pair PatchPath Patch] -> Patch -> Patch',
  (project, leafPatches, patch) =>
+    // TODO: check for marker instead of hasImpl?


See above. There should be just one proper way to check impl.

brusherru · 2018-12-13T14:40:52Z

About UX:

It would be awesome if I can add a column or row without manually moving cells.
The first row could have special styles
Selecting cells with Shift key will be handy ;)

And about overflow and scrollbars, note that grid can overflow in both ways: vertical / horizontal ;)

nkrkv · 2018-12-18T08:43:23Z

Looks nice, but I'm experiencing some strange behavior.

Steps to reproduce

1. Drill down to `xod/math/abs` tabtests

2. Double click the empty cell to the right of 0 on the second line to invoke the input

Expected behavior

* No jumps/resizes are done, the content starts editing inline literally

Actual behavior

* The cell becomes ~50px wider

* The cell becomes ~4px taller

The behavior is still here on Linux. Interesting thing: I can shut it up if add:

.data-grid-container .data-grid .cell > input {
  width: 140px;
}

140px is a measured width of a .cell in the normal state.

nkrkv · 2018-12-18T08:45:06Z

The table looks more organized if the very first row is rendered center-aligned and in bold:

.tabtest-editor tr:first-child .cell {
    font-weight: bold;
    text-align: center;
}

knopki · 2018-12-18T08:54:47Z

IMHO, "tabt..." looks not very nice. There is no easy way to make node two-cells-wide or just name it "tests"?

nkrkv · 2018-12-18T08:54:58Z

May I suggest alternate coloring?

It’s 2px borders of the color matching background $chrome-bg and foreground of $dark-light. Selection borders should be adjusted as well.

Renew

nkrkv · 2018-12-18T08:57:49Z

IMHO, "tabt..." looks not very nice. There is no easy way to make node two-cells-wide or just name it "tests"?

It’s a common pain for marker nodes in XOD. Eventually, we’ll implement auto-growth (#937) and the problem goes away. As a temporary solution we might make all markers two-three cells in width and/or manually resizable. That’s a question for a separate PR though.

nkrkv · 2018-12-18T08:58:51Z

packages/xod-client/src/core/styles/abstracts/mixins.scss

@@ -37,3 +37,23 @@
    background-color: #444444;
  }
 }
+
+@mixin styled-scrollbar {


👍 for utilizing mixins

nkrkv · 2018-12-18T09:00:43Z

packages/xod-client/src/core/styles/components/ReactDatasheet.scss

+  cursor: cell;
+  background-color: $input-bg;
+  color: $input-color-text;
+  transition : background-color 500ms ease;


Suggested change

transition : background-color 500ms ease;

transition : background-color 100ms ease;

500ms is more than human reaction time, so looks boring. Animations look snappier when done in 50-200ms

nkrkv · 2018-12-18T09:02:14Z

packages/xod-client/src/editor/components/TabtestEditor.jsx

+          valueRenderer={R.prop('value')}
+          overflow={'nowrap'}
+          onCellsChanged={(changes, additions = []) => {
+            R.compose(


Can you please extract it to a function? Now we have a complex nested function right inside deeply nested React component.

nkrkv · 2018-12-18T09:09:05Z

Also, what’s for the past comments about a single way to check whether a C++ implementation is present?

nkrkv · 2018-12-18T09:10:17Z

…and I suggest making cell editor inputs left-aligned. Otherwise experience is strange when I’m entering a long comment which overflows single cell boundaries

brusherru · 2018-12-21T13:51:51Z

🐛 Can't copy-paste markers tabtest and not-implemented-in-xod 🔥

brusherru · 2018-12-21T13:54:07Z

Bad UX
When I write a wide tabtest and horizontal scrollbar has appeared, after any change in the right column and press enter key horizontal scrollbar position is dropped and I have to scroll again...

Also, navigating with arrow keys are not scrolls the window, so I can select something outside the view area. When I start editing it — it scrolls.

Also if I might change column width, it can fit the window :)

evgenykochetkov · 2018-12-21T17:06:17Z

Fixed copy-pasting bug, thanks!

Navigating with arrow keys should be available in the very near future: nadbm/react-datasheet#92

Not sure what you mean by

Also if I might change column width, it can fit the window :)

Do you want resizable columns?

brusherru · 2018-12-21T17:20:53Z

@evgenykochetkov yep, I'm about resizable columns :) But, actually, it's out of scope 🙈
And it's not so trivial... where to store column width? The easiest and very questionable solution that I know is to store it right in local storage like a Map PatchPath [WidthInPx].

brusherru

LGTM, but left some suggestions about styling.
Also, it will be good to add selecting all with Cmd+A and select cells with the pressed Shift key, but it could be a tweak in another PR in nearest future.

packages/xod-client/src/core/styles/components/ReactDatasheet.scss

nkrkv

LGTM 🔥 for the release of tabtest

…ing `patch.test.tsv` attachments

…ch.test.tsv` attached

evgenykochetkov self-assigned this Dec 7, 2018

evgenykochetkov requested review from nkrkv and knopki December 7, 2018 11:47

evgenykochetkov force-pushed the feat-tabtest-markers branch 4 times, most recently from f3f669b to dc9150e Compare December 10, 2018 17:47

nkrkv reviewed Dec 12, 2018

View reviewed changes

nkrkv previously requested changes Dec 12, 2018

View reviewed changes

evgenykochetkov force-pushed the feat-tabtest-markers branch 4 times, most recently from af783e2 to 3b54e62 Compare December 14, 2018 17:03

evgenykochetkov requested a review from nkrkv December 17, 2018 10:01

evgenykochetkov force-pushed the feat-tabtest-markers branch 2 times, most recently from 2548323 to 2102a10 Compare December 17, 2018 10:11

nkrkv requested changes Dec 18, 2018

View reviewed changes

evgenykochetkov force-pushed the feat-tabtest-markers branch 3 times, most recently from e671456 to 3668c4c Compare December 19, 2018 16:34

evgenykochetkov force-pushed the feat-tabtest-markers branch from 3668c4c to 48e223c Compare December 19, 2018 16:51

brusherru reviewed Dec 24, 2018

View reviewed changes

packages/xod-client/src/core/styles/components/ReactDatasheet.scss Show resolved Hide resolved

packages/xod-client/src/core/styles/components/ReactDatasheet.scss Outdated Show resolved Hide resolved

packages/xod-client/src/core/styles/components/ReactDatasheet.scss Show resolved Hide resolved

brusherru approved these changes Dec 25, 2018

View reviewed changes

knopki approved these changes Dec 25, 2018

View reviewed changes

nkrkv approved these changes Dec 25, 2018

View reviewed changes

evgenykochetkov force-pushed the feat-tabtest-markers branch from 8885bab to 30e6e2f Compare December 25, 2018 16:30

evgenykochetkov added 2 commits December 25, 2018 19:43

feat(xod-project, xod-client): add tabtest marker that allows manag…

7b8bffc

…ing `patch.test.tsv` attachments

feat(stdlib): place tabtest markers for all patches that had a `pat…

048eef7

…ch.test.tsv` attached

evgenykochetkov force-pushed the feat-tabtest-markers branch from 30e6e2f to 048eef7 Compare December 25, 2018 16:43

evgenykochetkov merged commit d9dd2bf into master Dec 25, 2018

evgenykochetkov deleted the feat-tabtest-markers branch December 25, 2018 17:25

evgenykochetkov mentioned this pull request Dec 26, 2018

Update changelog for upcoming 0.26.0 #1604

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tabtest viewer to IDE #1585

Add tabtest viewer to IDE #1585

evgenykochetkov commented Dec 7, 2018

nkrkv commented Dec 12, 2018

nkrkv left a comment

nkrkv Dec 12, 2018

nkrkv Dec 12, 2018

nkrkv Dec 12, 2018

brusherru commented Dec 13, 2018 •

edited

Loading

nkrkv commented Dec 18, 2018

Steps to reproduce

Expected behavior

Actual behavior

nkrkv commented Dec 18, 2018

knopki commented Dec 18, 2018

nkrkv commented Dec 18, 2018

nkrkv commented Dec 18, 2018

nkrkv Dec 18, 2018

nkrkv Dec 18, 2018

nkrkv Dec 18, 2018

nkrkv commented Dec 18, 2018

nkrkv commented Dec 18, 2018

brusherru commented Dec 21, 2018 •

edited

Loading

brusherru commented Dec 21, 2018 •

edited

Loading

evgenykochetkov commented Dec 21, 2018

brusherru commented Dec 21, 2018

brusherru left a comment

nkrkv left a comment

	transition : background-color 500ms ease;
	transition : background-color 100ms ease;

Add tabtest viewer to IDE #1585

Add tabtest viewer to IDE #1585

Conversation

evgenykochetkov commented Dec 7, 2018

nkrkv commented Dec 12, 2018

Steps to reproduce

Expected behavior

Actual behavior

Steps to reproduce

Expected behavior

Actual behavior

nkrkv left a comment

Choose a reason for hiding this comment

nkrkv Dec 12, 2018

Choose a reason for hiding this comment

nkrkv Dec 12, 2018

Choose a reason for hiding this comment

nkrkv Dec 12, 2018

Choose a reason for hiding this comment

brusherru commented Dec 13, 2018 • edited Loading

nkrkv commented Dec 18, 2018

Steps to reproduce

Expected behavior

Actual behavior

nkrkv commented Dec 18, 2018

knopki commented Dec 18, 2018

nkrkv commented Dec 18, 2018

nkrkv commented Dec 18, 2018

nkrkv Dec 18, 2018

Choose a reason for hiding this comment

nkrkv Dec 18, 2018

Choose a reason for hiding this comment

nkrkv Dec 18, 2018

Choose a reason for hiding this comment

nkrkv commented Dec 18, 2018

nkrkv commented Dec 18, 2018

brusherru commented Dec 21, 2018 • edited Loading

brusherru commented Dec 21, 2018 • edited Loading

evgenykochetkov commented Dec 21, 2018

brusherru commented Dec 21, 2018

brusherru left a comment

Choose a reason for hiding this comment

nkrkv left a comment

Choose a reason for hiding this comment

brusherru commented Dec 13, 2018 •

edited

Loading

brusherru commented Dec 21, 2018 •

edited

Loading

brusherru commented Dec 21, 2018 •

edited

Loading