~whereswaldon/arbor-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
2 2

[PATCH sprig] feat: move collapse children button out of overflow menu

Details
Message ID
<20210406034446.146758-1-td3of4@gmail.com>
DKIM signature
missing
Download raw message
Patch: +26 -6
---
 icons/icons.go | 10 ++++++++++
 reply-view.go  | 22 ++++++++++++++++------
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/icons/icons.go b/icons/icons.go
index f711939..1d000ca 100644
--- a/icons/icons.go
+++ b/icons/icons.go
@@ -89,3 +89,13 @@ var SubscriptionIcon *widget.Icon = func() *widget.Icon {
	icon, _ := widget.NewIcon(icons.CommunicationImportContacts)
	return icon
}()

var CollapseIcon *widget.Icon = func() *widget.Icon {
    icon, _ := widget.NewIcon(icons.NavigationUnfoldLess)
    return icon
}()

var ExpandIcon *widget.Icon = func() *widget.Icon {
    icon, _ := widget.NewIcon(icons.NavigationUnfoldMore)
    return icon
}()
diff --git a/reply-view.go b/reply-view.go
index 9bf1a77..360e614 100644
--- a/reply-view.go
+++ b/reply-view.go
@@ -269,6 +269,11 @@ func (c *ReplyListView) AppBarData() (bool, string, []materials.AppBarAction, []

func (c *ReplyListView) getContextualActions() ([]materials.AppBarAction, []materials.OverflowAction) {
	th := c.Theme().Current().Theme
	focusedID := c.FocusTracker.Focused.ID()
    collapseIcon := icons.CollapseIcon
    if anchor := c.HiddenTracker.IsAnchor(focusedID); anchor {
        collapseIcon = icons.ExpandIcon
    }
	return []materials.AppBarAction{
			materials.SimpleIconAction(
				th,
@@ -288,12 +293,16 @@ func (c *ReplyListView) getContextualActions() ([]materials.AppBarAction, []mate
					Tag:  &c.CreateReplyButton,
				},
			),
		}, []materials.OverflowAction{
			{
				Name: "Hide/Show descendants",
				Tag:  &c.HideDescendantsButton,
			},
		}
            materials.SimpleIconAction(
                th,
                &c.HideDescendantsButton,
                collapseIcon,
                materials.OverflowAction{
                    Name: "Hide/Show descendants",
                    Tag:  &c.HideDescendantsButton,
                },
            ),
		}, []materials.OverflowAction{}
}

func (c *ReplyListView) triggerReplyContextMenu(gtx layout.Context) {
@@ -611,6 +620,7 @@ func (c *ReplyListView) Update(gtx layout.Context) {
	}
	if overflowTag == &c.HideDescendantsButton || c.HideDescendantsButton.Clicked() {
		c.toggleDescendantsHidden()
        c.triggerReplyContextMenu(gtx)
	}
	c.processMessagePointerEvents(gtx)
	c.refreshNodeStatus(gtx)
-- 
2.31.0

[sprig/patches/debian.yml] build success

builds.sr.ht <builds@sr.ht>
Details
Message ID
<CAGC12KS3JRB.U2L1CPANIIV7@cirno>
In-Reply-To
<20210406034446.146758-1-td3of4@gmail.com> (view parent)
DKIM signature
missing
Download raw message
sprig/patches/debian.yml: SUCCESS in 3m55s

[feat: move collapse children button out of overflow menu][0] from [Thom Dickson][1]

[0]: https://lists.sr.ht/~whereswaldon/arbor-dev/patches/21829
[1]: td3of4@gmail.com

✓ #478468 SUCCESS sprig/patches/debian.yml https://builds.sr.ht/~whereswaldon/job/478468
Details
Message ID
<CAHV2WUKLDPV.Z77QERT9ZV02@vendetta>
In-Reply-To
<20210406034446.146758-1-td3of4@gmail.com> (view parent)
DKIM signature
missing
Download raw message
Hi Thom,

Thanks for submitting this! I have verified locally that it does what I expect,
and I think it looks (visually) great! However, I think we can probably do
something a bit different with the implementation.

If you look here:

https://git.sr.ht/~eliasnaur/gio-example/tree/main/item/x/component/main.go#L679

you can see that this is a SimpleIconButton, but we customize its foreground
color at layout time by checking a (in the case global) variable. We could use
 a similar approach to this to choose the icon we use as part of the Layout
function of the overflow button. It makes the icon change instantly (when the
status of the node changes to/from being an anchor) instead of requiring the
context menu to be re-triggered.

Technically, what I'm proposing is less efficient, as we choose the icon each
frame instead of once per display-of-menu, but that overhead is negligible,
and I think it's easy to understand if written that way.

Does what I'm proposing make sense to you?

Cheers,
Chris


On Mon Apr 5, 2021 at 11:44 PM EDT, Thom Dickson wrote:
> ---
> icons/icons.go | 10 ++++++++++
> reply-view.go | 22 ++++++++++++++++------
> 2 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/icons/icons.go b/icons/icons.go
> index f711939..1d000ca 100644
> --- a/icons/icons.go
> +++ b/icons/icons.go
> @@ -89,3 +89,13 @@ var SubscriptionIcon *widget.Icon = func()
> *widget.Icon {
> icon, _ := widget.NewIcon(icons.CommunicationImportContacts)
> return icon
> }()
> +
> +var CollapseIcon *widget.Icon = func() *widget.Icon {
> + icon, _ := widget.NewIcon(icons.NavigationUnfoldLess)
> + return icon
> +}()
> +
> +var ExpandIcon *widget.Icon = func() *widget.Icon {
> + icon, _ := widget.NewIcon(icons.NavigationUnfoldMore)
> + return icon
> +}()
> diff --git a/reply-view.go b/reply-view.go
> index 9bf1a77..360e614 100644
> --- a/reply-view.go
> +++ b/reply-view.go
> @@ -269,6 +269,11 @@ func (c *ReplyListView) AppBarData() (bool, string,
> []materials.AppBarAction, []
>  
> func (c *ReplyListView) getContextualActions()
> ([]materials.AppBarAction, []materials.OverflowAction) {
> th := c.Theme().Current().Theme
> + focusedID := c.FocusTracker.Focused.ID()
> + collapseIcon := icons.CollapseIcon
> + if anchor := c.HiddenTracker.IsAnchor(focusedID); anchor {
> + collapseIcon = icons.ExpandIcon
> + }
> return []materials.AppBarAction{
> materials.SimpleIconAction(
> th,
> @@ -288,12 +293,16 @@ func (c *ReplyListView) getContextualActions()
> ([]materials.AppBarAction, []mate
> Tag: &c.CreateReplyButton,
> },
> ),
> - }, []materials.OverflowAction{
> - {
> - Name: "Hide/Show descendants",
> - Tag: &c.HideDescendantsButton,
> - },
> - }
> + materials.SimpleIconAction(
> + th,
> + &c.HideDescendantsButton,
> + collapseIcon,
> + materials.OverflowAction{
> + Name: "Hide/Show descendants",
> + Tag: &c.HideDescendantsButton,
> + },
> + ),
> + }, []materials.OverflowAction{}
> }
>  
> func (c *ReplyListView) triggerReplyContextMenu(gtx layout.Context) {
> @@ -611,6 +620,7 @@ func (c *ReplyListView) Update(gtx layout.Context) {
> }
> if overflowTag == &c.HideDescendantsButton ||
> c.HideDescendantsButton.Clicked() {
> c.toggleDescendantsHidden()
> + c.triggerReplyContextMenu(gtx)
> }
> c.processMessagePointerEvents(gtx)
> c.refreshNodeStatus(gtx)
> --
> 2.31.0
Reply to thread Export thread (mbox)