cells.Sequence of clickables is laid out incorrectly

[braxtonmckee]

See the list of pages we can navigate to in the playground demo, which looks like this:

The current API expects these things to be laid out vertically (one on top of the other). I think it would be good to allow both Sequence and SubscribedSequence to have a 'horiztonal' mode (where we do what's being done here) and overload ">>" to invoke this behavior, but the current code expects Sequence and "+" on cells, which makes a Sequence, to lay things out vertically.

[darth-cheney]

Two things.

Fixing the playground issue (should) be easy enough.

If we want to refactor Sequence like you suggest -- and it sounds like a good idea -- you might end up having to slightly modify a bunch of existing Cells declarative code at some point. One example is "rows of buttons" like the one in the screenshot here:

The blue buttons are actually enclosed in a Sequence which, like you say, should be displayed vertically. I'm still trying to figure out what is actually making them horizontal, but I suspect it has something to do with the bootstrap button classes that we are using and that they tell themselves to display inline.

If we switched to the new type of Sequence, you'd have to explicitly pass the direction="horizontal" arg in any declaration just to be safe.

[braxtonmckee]

They're horizontal because they contain a 'Span' cell (that I'd like to get rid of, along with 'nowrap()). I'm happy refactoring to support this since the current pattern is completely unusable.

Also, I almost never create 'Sequence' objects directly - I use the add method, which is smart enough to handle a Sequence + Sequence or Sequence + other cell. So if we retain the behavior that "+" means vertical and ">>" means 'horizontal' then most code should work unmodified and i'll just have to go through and find the places where we're using 'nowrap' or some other trickery to make this work correctly and replace them with the right behavior.

On Wed, Jul 17, 2019 at 11:53 AM Eric Gade notifications@github.com wrote:

Two things.

Fixing the playground issue (should) be easy enough.

If we want to refactor Sequence like you suggest -- and it sounds like a good idea -- you might end up having to slightly modify a bunch of existing Cells declarative code at some point. One example is "rows of buttons" like the one in the screenshot here:

[image: Screenshot from 2019-07-17 11-50-46] https://user-images.githubusercontent.com/1640299/61390658-341fdc80-a889-11e9-9a56-ca6cf8773a6e.png

The blue buttons are actually enclosed in a Sequence which, like you say, should be displayed vertically. I'm still trying to figure out what is actually making them horizontal, but I suspect it has something to do with the bootstrap button classes that we are using and that they tell themselves to display inline.

If we switched to the new type of Sequence, you'd have to explicitly pass the direction="horizontal" arg in any declaration just to be safe.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/189?email_source=notifications&email_token=AB6OHBAYSJI4UARG2K76AQDP74553A5CNFSM4IEGCQJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2E3JEY#issuecomment-512341139, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBCZIFWV433W4P4PAELP74553ANCNFSM4IEGCQJQ .

[darth-cheney]

Ok, this should be pretty straightforward then. I have confirmed what you've said: nowrap() is called on Clickables and it appends a display: inline-block to the clickable div, which cancels any flexbox formatting on the parent element. I'm going to remove this from just Clickable for now (@dkrasner I know you are working on removing some of these styles and we might overlap just a bit). This means there might be some goofy looking stacked buttons in the UI for the interim.

Additionally, since Sequence now bears the burden of saying whether its content items will "wrap" or not (ie punch down to the next line in cases of overflow), we need to define how SequenceC = SequenceA + SequenceB should behave. For the moment, I am setting the wrap property of SequenceC to be equal to that of SequenceB (and not of the original SequenceA). Does that make sense?

UPDATE: Looks like much of this is actually wrapped up in the current explicit styling / divStyle stuff that Daniel is working on, so I think we should put the Sequence refactor on hold for a bit until that all becomes clear.

[braxtonmckee]

SequenceA + SequenceB should concatenate the item lists, and all items should be on separate lines (stacked vertically).

I think it might actually be better to create a "HorizontalSequence" cell that gets created by '>>'. That way it's easier to track how the operators compose:

Cell + Cell = Sequence Sequence + Cell = Sequence Sequence + Sequence = Sequence Cell + Sequence = Sequence

Cell >> Cell = HorizontalSequence HorizontalSequence >> Cell = HorizontalSequence ... etc ...

with the idea that to 'Sequence', 'HorizontalSequence' is just a Cell, and vice versa. That is, (a + b) >> (c + d) should show up with 'a/b' next to 'c/d'.

On Wed, Jul 17, 2019 at 12:21 PM Eric Gade notifications@github.com wrote:

Ok, this should be pretty straightforward then. I have confirmed what you've said: nowrap() is called on Clickables and it appends a display: inline-block to the clickable div, which cancels any flexbox formatting on the parent element. I'm going to remove this from just Clickable for now (@dkrasner https://github.com/dkrasner I know you are working on removing some of these styles and we might overlap just a bit). This means there might be some goofy looking stacked buttons in the UI for the interim.

Additionally, since Sequence now bears the burden of saying whether its content items will "wrap" or not (ie punch down to the next line in cases of overflow), we need to define how SequenceC = SequenceA + SequenceB should behave. For the moment, I am setting the wrap property of SequenceC to be equal to that of SequenceB (and not of the original SequenceA). Does that make sense?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/189?email_source=notifications&email_token=AB6OHBCJ3GMZGFIU6XEIGHTP75BKBA5CNFSM4IEGCQJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2E6EYQ#issuecomment-512352866, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBHLW63DURCXH7TKUYTP75BKBANCNFSM4IEGCQJQ .

[dkrasner]

I added a split argument to sequence and it works fine. However, in updating the web service test layout there is a Sequence cell/component that is a second child from <div id='page_root'> and I can't figure where it is defined. I tried going through the logic of how ActiveWebService creates the main page, and I see that the main/initial layout is defined in this cells.withRoot object, which in turn is takes a lambda func return this func, which return a mainBar from utils.

mainBar is actually a tuple which is not just the header bar but the entire display. BUT the cells/elements returned in this tuple are already children of the Sequence i mentioned above. So somewhere between cells.withRoot and mainBar this Sequence element is defined, and there is no way to step through it to see if I am simply missing something. If knew where this layout was defined I could simply flip the split param to horizontal and all would be well.

Maybe you know.

btw this is all on the daniel-divstyle-remove branch.

With that said, we probably need to rewrite the web test at some point to uncouple the server, communication and cell/render code but that's another conversation.

BUT What is the rationale for keeping both a Sequence and the SplitView cell.

They are both made to take a list of children and display them accordingly (horizontal/vertical). Why not remove all Sequences and replaces them with SplitViews?

[braxtonmckee]

Sequences don’t fill space. They just stack their items too to bottom. SpliView takes the space and apportions it between two or more subcells to a given proportion

Sent from my iPhone

On Jul 18, 2019, at 6:05 AM, dkrasner notifications@github.com wrote:

What is the rationale for keeping both a Sequence and the SplitView cell.

They are both made to take a list of other divs and display them accordingly (horizontal/vertical). Why not remove all Sequences and replaces them with SplitViews

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[dkrasner]

i understand the idea, but these are just DOM elements that display children and fill up space - one is set to 2 children with explicit proportions and one is not

You can have a nice split view with as many children as you want and a sequence of proportions that default to all the same.

[braxtonmckee]

That's an implementation detail. If you want the Python functions to use the same component in the client to implement the behavior that's fine. But from the API's point of view, "Sequence" creates vertically stacked cells that take up the minimum space they can (like lines of text) and SplitView creates a fixed number of sub panes and fills the space allotted to it.

On Thu, Jul 18, 2019 at 8:14 AM dkrasner notifications@github.com wrote:

i understand the idea, but these are just DOM elements that display children and fill up space - one is set to 2 children with explicit proportions and one is not

You can have a nice split view with as many children as you want and a sequence of proportions that default to all the same.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/189?email_source=notifications&email_token=AB6OHBCZEMC66WN5EKD3IWDQABNDFA5CNFSM4IEGCQJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2II6BA#issuecomment-512790276, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBHBZRBSXCR2EX6I5S3QABNDFANCNFSM4IEGCQJQ .

[dkrasner]

fair, i added a split arg to Sequence which defaults to a horizontal split, i.e. a vertical stack.

This is fine for now and should not affect anything, other than the fact that Sequence styling is now moved to the client side and is inline-flex (as I think it it intended to be).

Down the line we can add a HorizontalSequence cell to clearly distinguish for the python dev what is happening.