Swap items in a list

I would like the user to be able to sort items in a list by selecting an item and moving it up or down one (or more) positions.

Using the functions lv_obj_move_foreground and lv_obj_move_background, you can move an item in a list to the first or last postion.

I would like to have a function added that swaps item[n] and item[n+1] or exchange items [n] and [m].

I created a c function, but to be able to use this functionality in MicroPython it probably should be part of the lvgl library

lv_obj_swap function

// #define MY_CLASS &lv_obj_class
static void lv_obj_swap(lv_obj_t* obj1, lv_obj_t* obj2)
{
    LV_ASSERT_OBJ(obj1, MY_CLASS)
    LV_ASSERT_OBJ(obj2, MY_CLASS)

    lv_obj_t* parent = lv_obj_get_parent(obj1);
    lv_obj_t* parent2 = lv_obj_get_parent(obj2);
    LV_ASSERT(parent == parent2);

    uint_fast32_t index1 = lv_obj_get_child_id(obj1);
    uint_fast32_t index2 = lv_obj_get_child_id(obj2);

    parent->spec_attr->children[index1] = obj2;
    parent->spec_attr->children[index2] = obj1;

    lv_event_send(parent, LV_EVENT_CHILD_CHANGED, obj2);
    lv_event_send(parent, LV_EVENT_CHILD_CHANGED, obj1);

    lv_obj_invalidate(parent);
}

static void lv_obj_swap_children(lv_obj_t* parent, uint32_t child1, uint32_t child2)
{
    LV_ASSERT_OBJ(parent, MY_CLASS);

    uint32_t cnt = lv_obj_get_child_cnt(parent);

    LV_ASSERT(cnt > child1);
    LV_ASSERT(cnt > child2);

    lv_obj_t* obj1 = parent->spec_attr->children[child1];
    lv_obj_t* obj2 = parent->spec_attr->children[child2];

    parent->spec_attr->children[child1] = obj2;
    parent->spec_attr->children[child2] = obj1;

    lv_event_send(parent, LV_EVENT_CHILD_CHANGED, obj2);
    lv_event_send(parent, LV_EVENT_CHILD_CHANGED, obj1);

    lv_obj_invalidate(parent);
}

Sample application

static lv_obj_t* list2;
static lv_obj_t* buttonColumn;

static lv_obj_t* currentButton = NULL;
static lv_obj_t* btnUp;
static lv_obj_t* btnDn;

static void event_handler(lv_event_t* e)
{
    lv_event_code_t code = lv_event_get_code(e);
    lv_obj_t* obj = lv_event_get_target(e);
    if (code == LV_EVENT_CLICKED)
    {
        LV_LOG_USER("Clicked: %s", lv_list_get_btn_text(list2, obj));

        if (currentButton == obj)
        {
            currentButton = NULL;
            lv_obj_add_state(btnUp, LV_STATE_DISABLED);
            lv_obj_add_state(btnDn, LV_STATE_DISABLED);
        }
        else
        {
            currentButton = obj;
        }
        lv_obj_t* parent = lv_obj_get_parent(obj);
        uint32_t i;
        for (i = 0; i < lv_obj_get_child_cnt(parent); i++)
        {
            lv_obj_t* child = lv_obj_get_child(parent, i);
            if (child == currentButton)
            {
                lv_obj_add_state(child, LV_STATE_CHECKED);
            }
            else
            {
                lv_obj_clear_state(child, LV_STATE_CHECKED);
            }
        }
    }
}

static void event_handler_mu(lv_event_t* e)
{
    lv_event_code_t code = lv_event_get_code(e);
    // lv_obj_t* obj = lv_event_get_target(e);
    if ((code == LV_EVENT_CLICKED) || (code == LV_EVENT_LONG_PRESSED_REPEAT))
    {
        if (currentButton == NULL) return;
        lv_obj_t* parent = lv_obj_get_parent(currentButton);
        uint_fast32_t i = lv_obj_get_child_id(currentButton);
        if (i > 0)
        {
            //lv_obj_swap(parent->spec_attr->children[i], parent->spec_attr->children[i - 1]);
            lv_obj_swap_children(parent, i, i - 1);
        }
    }
}

static void event_handler_dn(lv_event_t* e)
{
    lv_event_code_t code = lv_event_get_code(e);
    // lv_obj_t* obj = lv_event_get_target(e);
    if ((code == LV_EVENT_CLICKED) || (code == LV_EVENT_LONG_PRESSED_REPEAT))
    {
        if (currentButton == NULL) return;
        lv_obj_t* parent = lv_obj_get_parent(currentButton);
        uint_fast32_t i = lv_obj_get_child_id(currentButton);
        if (i < lv_obj_get_child_cnt(parent) - 1)
        {
            //lv_obj_swap(parent->spec_attr->children[i], parent->spec_attr->children[i + 1]);
            lv_obj_swap_children(parent, i, i + 1);
        }
    }
}


void lv_example_list_2(void)
{
    /*Create a list*/
    list2 = lv_list_create(lv_scr_act());
    lv_obj_set_size(list2, lv_obj_get_width(lv_scr_act()) - 90, lv_obj_get_height(lv_scr_act()) - 10);
    lv_obj_set_align(list2, LV_ALIGN_TOP_LEFT);
    lv_obj_set_pos(list2, 5, 5);
    lv_obj_set_flex_flow(list2, LV_FLEX_FLOW_COLUMN);

    /*Add buttons to the list*/
    lv_obj_t* btn;

    for (int i = 0; i < 10; i++)
    {
        char szBuf[100];
        sprintf(szBuf, " Item %d ", i);
        btn = lv_btn_create(list2);
        lv_obj_t* lab = lv_label_create(btn);
        lv_label_set_text(lab, szBuf);
        lv_obj_add_event_cb(btn, event_handler, LV_EVENT_CLICKED, NULL);
    }

    buttonColumn = lv_obj_create(lv_scr_act());
    lv_obj_set_size(buttonColumn, 80, lv_obj_get_height(list2));
    lv_obj_set_align(buttonColumn, LV_ALIGN_TOP_RIGHT);
    lv_obj_set_pos(buttonColumn, -5, 5);
    lv_obj_set_flex_flow(buttonColumn, LV_FLEX_FLOW_COLUMN);

    btnUp = lv_list_add_btn(buttonColumn, LV_SYMBOL_UP, NULL);
    lv_obj_add_event_cb(btnUp, event_handler_mu, LV_EVENT_ALL, NULL);
    btnDn = lv_list_add_btn(buttonColumn, LV_SYMBOL_DOWN, NULL);
    lv_obj_add_event_cb(btnDn, event_handler_dn, LV_EVENT_ALL, NULL);
}

Hi,

It looks like a very reasonable feature.

Why do we need lv_obj_swap_children? Can’t lv_obj_swap do its job too?

Hi,

I created two versions of the function, not sure which one is better (or has a better fit in the overall architecture).

one of them will do i think, and is also more easy to maintain.

in lv_obj_swap i have an ASSERT on the parents, not sure if there is a need for different parents (and if that would be mandatory. if more parents don’t have to be the same, the next lines must be added on the end of the function:

if (parent != parent2)
{
   lv_obj_invalidate(parent2);
}

I like the lv_obj_swap() name.

Can you send a Pull request with this new feature?

I can try, but i never did this before
so it will be a new experience for me :slight_smile: but it probably will take a few hours.

BTW do you want the version with one parent or with multiple parents?

done!, although i forgot the changelog.
there is no MicroPython sample, but i’ll add it when there is a MP version.

something like obj.swap(other) i think.

You did it very well. Special thanks for updating the docs. :slight_smile:

Perfect.

I had some minor comments on the PR. You can push new commits to your branch and they will appear in the PR.

Hi again,

do you think there is a need for functions like lv_obj_move_up/down to (better naming is welcome)?

==> lv_obj_swap(obj[x], obj[x+1]) and lv_obj_swap(obj[x], obj[x-1]) (after testing for top/bottom of list).

if so, i’ll be glad to do an other pull request for them,

It sounds reasonable. A PR would be welcome. :slight_smile:

Meanwhile, I added a few minor adjustments to your example.

looks fine to me, i noticed that you put the
‘{’ on the same line as ‘if’ (and probably ‘while’), i’ll do the same in the future.

are the names lv_obj_move_up(obj) and lv_obj_move_down(obj) ok for you or do you have a better name?

and how about returning an bool (alike) for moved/not moved (or does no-one care)?

Thanks! :slight_smile:

Sounds good to me.

I think we should not add return value without a good reason because once we will have a good reason to add a return value we can’t add it without API break. :smiley:

Hi,

while writing the lv_obj_move_up/down functions i noticed that in the lv_obj_move_background function, there are 2 calls to lv_obj_invalidate(parent). the 2nd one is obvious but i wondered why the first one is done.

is it meant to be there or is it an unneeded command?

and when it is meant to be there, i should probably also add it to the lv_obj_swap, and lv_obj_move_up/down functions.

void lv_obj_move_background(lv_obj_t * obj)
{
    LV_ASSERT_OBJ(obj, MY_CLASS);

    lv_obj_t * parent = lv_obj_get_parent(obj);

    lv_obj_invalidate(parent);  <-- this one

    int32_t i;
    for(i = lv_obj_get_child_id(obj); i > 0; i--) {
        parent->spec_attr->children[i] = parent->spec_attr->children[i-1];
    }
    parent->spec_attr->children[0] = obj;

    /*Notify the new parent about the child*/
    lv_event_send(parent, LV_EVENT_CHILD_CHANGED, obj);

    lv_obj_invalidate(parent);
}

Hi,

TL;DR it’s a mistake, one invalidation should be enough.

It’s really not obvious but in some cases changing the order of the objects can change the size of the parent. E.g. there is a layout like this:

||-A-||-----B-----||
||-----C-----|     |

If we swap A and C, C + B won’t fit in a row and moves A into the third line which makes the parent higher. (if it has LV_SIZE_CONTENT height)

||-----C-----|    |
||-----B-----|    |
||-A-|            |

So both the original and the new area of the parent should be invalidated.

But thinking about it again it seems it’d be enough invalidate once because if the parent size changes it will handle its invalidation in “size change handlers”. :smiley:

So if you send a new PR, please remove the first invalidation too. :slightly_smiling_face:

Hi,

I’ve done a PR for the new functions.
How can I add the functions to MicroPython, once approved?
do I automatically get a new version of lvgl when I fetch lv_micropython (or when will the latest version of lvgl be merged into lv_micropython)?

I implemented a function similar to this just last week! Can I suggest that lv_obj_swap is only one possible manipulation however?

For example, I implemented lv_obj_shuffle(lv_obj_t* obj, int to_index), which moves the intervening objects over by one slot (up or down) and moves the specified object into the new vacant place.

Before moving too far down the path of adding a plethora of specific child-ordering manipulation functions, it might be good to see if they can be generalised to reduce the number?

I also had noticed that there was an extra invalidation, but hadn’t got around to asking why it was there :slight_smile:

When @amirgon bumps the LVGL submodule in lv_micropython you will get a new version. Alternatively, you can cd lib/lv_bindings/lvgl && git checkout master && git pull. This works most of the time because we have a CI test which verifies MicroPython compatibility with new changes.

What about lv_obj_swap_index(parent, obj1_index, obj2_index)? It seems quite generic to me and a straightforward extension of lv_obj_swap().

It seems to me that swapping two objects is an unusual case. More often you want to manipulate one object at a time, e.g. insert at a specific position, or move it to a specific position from wherever it is. For that, all you need (in addition to lv_obj_move_foreground and lv_obj_move_background) is lv_obj_move_above(obj_to_move, obj_to_be_above) or a lv_obj_move_to_index operation. That allows swapping in two lines of code, but supports the more common list-insert and shuffle operations directly.

I’d rather call it lv_obj_move_after and lv_obj_move_after_id but in general I like the idea.

@Karijn What do you think? Do you also think it can be cleaner to combine these functions?

I’m happy with that, except “after” is below, and I’d probably use “before” (above) more readily.