search

raimohanska / ourboard

An online whiteboard
Other
763 stars 58 forks source link

Implement aligning and distributing items from context menu #143

Closed mtreinik closed 3 years ago

mtreinik commented 3 years ago

I put my changes in a branch because I'm unsure about my usage of L.view and Property.

Stuff in ContextMenuView.moveFocusedItems isn't probably very idiomatic. I'm suspecting there is a smarter way to avoid those ?.get() || 0s.

raimohanska commented 3 years ago

Seems to work! I think it's great to have tools for organizing content.

Am a bit hesitant about the amount of items now in the context menu as well as the use case for all of these options though. Then there's the tendency of notes overlapping as they are only moved on one axis at the time.

Could we maybe come up with 1-2 a bit more opinionated layouts (to avoid UI confusion) that would avoid the overlap issues?

raimohanska commented 3 years ago

Codewise I think we could separate this into a separate widgetCreator from colors, like done in menuFontSizes.

raimohanska commented 3 years ago

And tests failing?

raimohanska commented 3 years ago

I don't see why you have ?.get() as L.view never returns undefined.

raimohanska commented 3 years ago

I see the need for || 0 because focusedItems can return zero items. However in moveFocusedItems you can do focusedItems.get() just once and then operate on pure values without having to use L.view at all.

raimohanska commented 3 years ago

You'll want to use view only if you need to return an observable result. In moveFocusedItems you can just .get() the values from observables once.

mtreinik commented 3 years ago

Thanks for the feedback!

Miro has a dropdown menu on the context menu for different alignment buttons. Maybe I'll experiment with a dropdown menu as well to reduce the size of context menu.

CI tests fail because of this (I don't know if it is related to my changes or not):

http://localhost:1337 timed out on retry 34 of 2
Error: Response code 404 (Not Found)

I'll see if I can refactor my implementation to avoid the unneeded ?. and view etc.