Closed rgoj closed 1 year ago
Great work getting this running on staging Romek! Here are the main things that stood out for me on an initial test - I know you're aware of some or perhaps all of them but wanted to give you the fresh eyes perspective.
ESSENTIAL BEFORE SHIPPING It's already so much better than what we have in production so I think we should aim to ship this ASAP. The only two things I could really justify as essential are:
1) ~the initial expansion of nodes to include the first level when a map loads~ and then I think we said we'd hide the expand/collapse all buttons (eventually we'll add expand all to the context menu on each node).
2) expanded view: circle names in leaf nodes have changed from white to black text and are hard to read on the circles with darker backgrounds.
NON-ESSENTIAL In rough priority order. Highest at the top.
Vertical spacing around multi-line items is less than around single-line ones. I don’t know if it’s just due to the spacing but it does look cluttered now that we show circle names over multiple lines. As a map gets more levels of indentation there will be more and more multi-line items. Making spacing consistent will definitely help, or perhaps we should keep everything on a single line and have horizontal scrolling.
Clicking a circle on the map does select the right item in the outliner, but it should also scroll to it and expand any parent circle if not visible.
Not actually about the outliner, but just noting it here: This might have gotten lost during the summer, but remember in Pavilion Gardens we spoke about not zooming in quite so far when a circle is selected, so you can see a bit more context around it. Did we settle on 75% or 85% or something else?
I wonder if the Edit option on the context menu is redundant since clicking the item in the outliner opens up the edit panel anyway and it only takes one more click on the outliner item to edit it so it’s very discoverable and not actually essential since you can edit the circle name in the details panel.
When inline editing, I think pressing enter should save and stop editing. I don’t think enter should create a new item - I think that would get annoying when people just want to change the name of a circle and instinctively press enter to save. Perhaps control+enter should save and create a new child at the same level?
~When inline editing a new circle name for the first time it’s initially labeled ‘undefined’ once you’re editing.~
Great work getting this running on staging Romek!
Thanks, Tom! 🙇🏻
Here are the main things that stood out for me on an initial test - I know you're aware of some or perhaps all of them but wanted to give you the fresh eyes perspective.
Brilliant, thank you, yes, that's exactly what we needed!
the initial expansion of nodes to include the first level when a map loads and then I think we said we'd hide the expand/collapse all buttons (eventually we'll add expand all to the context menu on each node).
The initial expansion I already completed yesterday after shipping to staging ☺️ I've crossed out from your post anything that is already completed so that it's easier to scan that!
expanded view: circle names in leaf nodes have changed from white to black text and are hard to read on the circles with darker backgrounds.
Oh, briliant, thank you for spotting!! This made me smile actually, because this has been happening for me locally for a long time (it's unrelated to the outliner or the architectural work) and I couldn't immediately see why - and didn't want to spend time investigating as it wasn't happening in production or on staging. Until now. And the fact it appeared on staging yesterday - right after I had to fix the build - makes it obvious where this is coming from! Shouldn't be a problem to fix, much appreciation for spotting, I didn't test the expanded view on staging yesterday! 🙏🏻
This finally replaces the old outliner with a notebits-based library, building on top of a chain of branches, the previous one being #837.
Here's an in-progress (not yet complete) list of things that need to be finished for this PR to be merged. I've not divided it yet into: must-be-done and someday-maybe tasks.
Completed tasks
[X] Selecting
[x] Editing
[x] Moving via drag & drop (timebox)
Completed smaller tasks
[REGRESSION]
Fix Cloudinary error from the image upload component that appeared after one of the couple Angular upgrades that went into this work[STYLES]
[TINY]
Fix icons misaligned with the circle around them - only in Maptio, this works in notebits.[STYLES]
Implement nice inline editing[STYLES]
Change colour for the selected outline item[STYLES]
Consider changing the shape of the highlight of the selected item[STYLES]
[SMALL]
Consider exactly what cursors we show when[STYLES]
Hovering over an outliner item should change the cursor to a pointer to indicate that the item can be clicked for a meaningful interaction?[STYLES]
Hovering over an outliner item should highlight the item just slightly[USABILITY]
[TINY]
Start with the first level expanded[USABILITY]
[TINY]
When the name of a circle isundefined
show an empty string[REGRESSION]
[TINY]
Expanded view: circle names in leaf nodes have changed from white to black text and are hard to read on the circles with darker backgrounds.[USABILITY]
Consider showing greyed out placeholder text like "New circle" to empty outliner nodes to make them more obvious[USABILITY]
[SMALL]
Standard users should be unable to make edits[STYLES]
[USABILITY]
[BUG]
[MEDIUM]
Vertical spacing part of: Improve handling of multiline items (replace input with textarea; improve margins/padding even when not editing; fix bug that happens when editing and pressing any of the buttons)[TECH DEBT]
[TINY]
Remove the old outliner when we're readyRemaining smaller tasks
Fast Follow-on
[USABILITY]
When a new sub-circle is added, it's (no longer?) showing on the map and in the circle details pane, let's change that[STYLES]
[TINY]
Significantly reduce the indent to make more space for the initiative names[STYLES]
[USABILITY]
[BUG]
[MEDIUM]
Other parts than vertical spacing: Improve handling of multiline items (replace input with textarea; improve margins/padding even when not editing; fix bug that happens when editing and pressing any of the buttons)[USABILITY]
[SMALL]
Pressing enter when editing should create a new item (this is almost a regression - with the old outliner it used to be very easy to create lots of circles at once and then start editing them - that was always a workaround, but it worked, whereas now there is no good way to do this - intentionally as the "right" solution, IMHO, is to do this on pressing Enter)[USABILITY]
[SMALL]
Pressing 'Esc' or 'Cmd/Ctrl Enter' while editing should allow you to exit editing currently you can only leave by clicking somewhere outside the outliner which is fiddly (it's debatable whether this is the right solution - OmniFocus does something different, Finder too, but their solutions are slightly trickier to implement in some ways)[USABILITY]
[TINY]
When the root circle is deleted and created from scratch, make sure the new outliner is updated and the new root circle selected[STYLES]
[SMALL]
Improve the drag and drop styles a bit (see also bigger task below) (timebox)[REGRESSION]
[SMALL]
While connecting the new outliner with the expanded map, I appear to have accidentally broken a part of the flow of opening and closing circles. Specifically, when you're zoomed in a few levels deep and click outside the currently selected circle, you should be zoomed out and see all the siblings of that circle (see behaviour in production). Instead, now (see behaviour on staging), you're zoomed out to the cover of the parent circle, making exploration a lot more annoying!Want to do
[TECH DEBT]
[SMALL]
Remove the cluttered code around the old outliner when we're ready[USABILITY]
When a new sub-circle is added, immediately trigger editing that circle[USABILITY]
Add expand/collapse children to the menu (this is to replace the "Expand/Collapse All" buttons)[STYLES]
Make sure the menu trigger button stays visible when the menu is open - even while you're no longer hovering over the item itself (which is what hides the menu trigger)[STYLES]
When a node doesn't have a toggle (i.e. it doesn't have any descendants), the left edge of the item highlight looks like it's sticking out a bit weirdly (because the toggle icon isn't there), what might be a nice way of addressing this?[TECH DEBT]
Move all of the new outliner-related template into a new component and the business logic into a new service to finish addressing the clutter insideBuildingComponent
and pave way for work on side panels (we could also do this work then to kick things off)Not for now
[REGRESSION]
[I18N]
[MEDIUM]
[DISCUSS!]
The tooltip for the "add sub-circle" button used to be translated, but the notebits outliner library doesn't support i18n yet! The way to solve this would be to move the labels out of the outliner library, to make them customisable in the project using the library, and that is the ultimate goal, but it's not possible yet. It's not trivial, but can be done. I'm hoping to do the bulk of the work in the future as part of notebits work though.[REGRESSION]
[USABILITY]
[DISCUSS!]
The old outliner persisted expansion state to local storage and recreated it when loading the map, I haven't reimplemented that (yet) as I don't think it's that important, especially if we open up the first level of nodes by default - still, not that hard to do![USABILITY]
Cut, copy & paste (optional now that we know drag & drop is working)[USABILITY]
PressingTab
andShift + Tab
should indent and outdent respectively[USABILITY]
When editing is triggered, show the cursor where the user clicked[STYLES]
[LARGE]
Improve the drag and drop styles - drag and drop with outliners is so tricky to get right, but it makes such a huge difference when the UX of this is really good! I am hoping to do this slowly as part of notebits work though, so perhaps we don't need to do this now (...though I would looove to!)