savedra1 / clipse

Configurable TUI clipboard manager for Unix
MIT License
314 stars 12 forks source link

[feat]: Pinned items view #25

Closed noornee closed 6 months ago

noornee commented 6 months ago

Hello, I'm interested in implementing the Pinned Items View feature. I'm not really familiar with bubble tea yet but I'm going to learn. i would appreciate clarification on the issue description. Does this entail creating a separate view dedicated to pinned items alongside the regular clipboard view?

savedra1 commented 6 months ago

Hi @noornee :wave: I'm happy to hear you'd like to implement this, it's been on the to-do list for a while now.

I'm not an expert with BubbleTea but I don't think a new view would be required. I was thinking an update could be made to the NewModel func, allowing a keybinding to be passed in as an arg, determining which items to return for the list view during the Update method. eg,

func NewModel(keyMsg string) model {
         var (
        delegateKeys = newDelegateKeyMap()
        listKeys     = newListKeyMap()
     ) 
        clipboardItems := config.GetHistory()
    var entryItems []list.Item

    for _, entry := range clipboardItems {
        shortenedVal := utils.Shorten(entry.Value)
        item := item{
            title:       shortenedVal,
            titleFull:   entry.Value,
            description: "Date copied: " + entry.Recorded,
            filePath:    entry.FilePath,
                        pinned: entry.Pinned
        }

        if (keyMsg == <not pinned>) {
              entryItems = append(entryItems, item)

       } else {
               // Code here for only appending entryItems where entry.Pinned == true       
       }

      "...... rest of func code"

       return model {
        list:         clipboardList,
        keys:         listKeys,
        delegateKeys: delegateKeys,
                pinned:   pinnedBool
    }

}

This approach would also require an update to the main model so that the app can toggle the pinned items on and off. eg,

type model struct {
    list         list.Model     
    keys         *listKeyMap    
    delegateKeys *delegateKeyMap 
        pinned   bool // this can be inspected during the Update method

}

Some way to differentiate the colors used for pinned items would also be a bonus.

These are my initial thoughts anyway, there could be some extra complexity I haven't considered yet so let me know if there's anything else I can try my best to clarify :smile:

Thank you!

noornee commented 6 months ago

These are my initial thoughts anyway, there could be some extra complexity I haven't considered yet so let me know if there's anything else I can try my best to clarify 😄

Thank you!

Whoaa, Thank you so much for this! you just made it easier for me to wrap my head around. Earlier when i was trying to implement the view function, i felt a bit frustrated because i couldnt wrap my head around how BubbleTea worked but looking at the code you just drafted out, it looks pretty simple. I'm going to implement this and give you a feedback on whatever complexity(i hope not) that might come up.

Thanks Again!

noornee commented 6 months ago

HIi @savedra1, The update works, the pinned items can be toggled on and off but the view isnt updated for the pinned items to display. I don't understand what i'm doing wrong :face_exhaling: Could you please provide further guidance ? Thank you ^^

video

out.webm

desc

I printed the status of m.pinned to stdout to see if the value is getting updated in real time. i was using the tab key binding to update the status of m.pinned

this is the relevant commit

savedra1 commented 6 months ago

Hi @noornee,

It looks like you're very close and the logic used looks great.

Looking at your commit, I can see the functionality for setting an item as pinned but I can't see another key binding for toggling the pinned items, this would be needed in the delegates as well as the keybind for 'pinning' individual items.

Also I think I may have lead you astray with the NewModel function (apologies). Now I'm thinking this doesn't actually need anything passing in as an arg, but the new delegate you add for 'toggling pinned items' can just add a new list directly to the model instead. Something like:

case key.Matches(msg, keys.togglePinned):
            // Toggle the showOnlyPinned flag
            m.showOnlyPinned = !m.showOnlyPinned

            // Create a filtered list of items based on the showOnlyPinned flag
            var filteredItems []list.Item
            for _, entry := range clipboardItems {
                // Create item...
                if !m.showOnlyPinned || entry.Pinned {
                    filteredItems = append(filteredItems, item)
                }
            }

            // Create a new list with the filtered items
            m.list = list.New(filteredItems, del, 0, 0)

            return nil 

With this approach newItemDelegate would need to inherit a pointer to the model and no changes would be needed to NewModel.

This is all theoretical and untested though so I'll take another look over the weekend in more detail but lmk if you manage to get it running :smile:

noornee commented 6 months ago

Hiii @savedra1

Gosh, Thank you soooo much for your detailed response and guidance. <3

I see what you mean about the need for having another key binding for toggling the pinned items and updating the delegates accordingly. I'll work on that

Regarding the elimination of the need passing for arguments to the NewModel function, it makes sense to me now hehe :sweat_smile: I'll make the necessary adjustments.

I'll start implementing these changes and will keep you updated on my progress. Thank youu once again for your guidance. I'll do my best to get it up and running tonight!

savedra1 commented 6 months ago

No worries! @noornee thank you for your contributions.

I actually had some time just now and found a method to toggle the pinned items on and off using the list.Model directly in d.UpdateFunc:

func (parentModel *model) newItemDelegate(keys *delegateKeyMap) list.DefaultDelegate //inhert parent

.....

case key.Matches(msg, keys.togglePin):
    if len(m.Items()) == 0 {
      keys.togglePin.SetEnabled(false)
    }

    if parentModel.togglePinned {
      parentModel.togglePinned = false
    } else {
      parentModel.togglePinned = true
    }
    clipboardItems := config.GetHistory() //* this could become a function
    var filteredItems []list.Item
    for _, entry := range clipboardItems {
      shortenedVal := utils.Shorten(entry.Value)
      item := item{
        title:       shortenedVal,
        titleFull:   entry.Value,
        description: "Date copied: " + entry.Recorded,
        filePath:    entry.FilePath,
        pinned:      entry.Pinned,
      } // * this could become a function

      if !parentModel.togglePinned {
        filteredItems = append(filteredItems, item)
      } else if entry.Pinned {
        filteredItems = append(filteredItems, item)
      }

    }

    for i := len(m.Items()) - 1; i >= 0; i-- { // clear all items (maybe an inbuilt func for this)
      m.RemoveItem(i)
    }

    for _, item := range filteredItems { // adds all required items
      m.InsertItem(len(m.Items()), item)
    }

This also required the following update to NewModel:

m := model{}
del := m.newItemDelegate(delegateKeys)

There may still be a more optimal way but this was working pretty well for me. Interested to see how you get on!

noornee commented 6 months ago

No worries! @noornee thank you for your contributions.

You're Welcome, it's all you. 🥹 you assisted me throughout the process and i really appreciate that.

There may still be a more optimal way but this was working pretty well for me. Interested to see how you get on!

Your solution is perfect 😭. you've saved me a lot of headaches. I just implemented your solution and everything works perfectly. The only thing left for me to do is to (oops, i accidentally closed the issue) do the bonus task of making the color of the pinned items more conspicuous

savedra1 commented 6 months ago

@noornee glad to hear it!

Yeah some sort of visual identifier would be good - also I think it'd be smart to prevent any pinned items from auto-deleting when max capacity is reached. That should be fairly simple logic in the 'add item' function to skip removal of the oldest item if pinned.

Happy to assist if needed :)

noornee commented 6 months ago

Hii @savedra1 , Gooday!

i want to update you on the little progress i've made and request your assistance on a few other things. here's how it looks atm, did some hacky things and i'm trying to improve it.

pin.webm

is it okay the way it currently looks? or is there some other style you suggest i go with?

I think it'd be smart to prevent any pinned items from auto-deleting when max capacity is reached. That should be fairly simple logic in the 'add item' function to skip removal of the oldest item if pinned