Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Section declaration is subpar #53

Closed
TonicArtos opened this issue Mar 4, 2015 · 7 comments
Closed

Section declaration is subpar #53

TonicArtos opened this issue Mar 4, 2015 · 7 comments

Comments

@TonicArtos
Copy link
Owner

Update

Okay, so the plan is to implement 2.a only.

Here are the changes that will happen.

Constructors:

// Basic constructor to use if you don't have the adapter constructed already. Using this
// constructor requires that RecyclerView#setLayoutManager(LayoutManager lm) must 
// be called before RecyclerView#setAdapter(Adapter a). Failure to do so will result in a
// runtime exception.
public LayoutManager(Context context);

// Standard constructor.
public LayoutManager(Context context, SectionAdapter adapter);

When RecyclerView#setAdapter(Adapter a); is called, if the adapter does not implement the SectionAdapter interface, a ExpectedSectionAdapterRuntimeException is thrown. It will print an error log message along the lines of, "Expected SectionAdapter but received Adapter instead."

If LayoutManager#onLayoutChildren(…) is called before LayoutManager has pulled the section data from an adapter, a AdapterNotSetRuntimeException is thrown. It will print an error log message along the lines of, "Adapter not set before layout. Instances of LayoutManager created with the LayoutManager(Context context) constructor must be added to a recycler view before the adapter is set."

New interface in LayoutManager:

interface SectionAdapter {
    List<Integer> getSectionStartPositions();
}

Proposal

I've been working on the data change animation support and one thing I've decided is that having to update section first position on all the items is really annoying. I mean, the data change notification range isn't even limited to the same section.

The biggest restriction on the API is that the first section item has to be found in some way. This is because section layouts are dynamic and derived from the header state. This means there is no loss to keeping section configuration in the layout params, which gives us the nicety of embedding configuration in xml.

Bearing that in mind, here are my thoughts on the API changes to reduce the adapter burden:

1. Let SuperSLiM find section start positions

1.a Section start indicator

Only the first item in a section is marked. SuperSLiM has to do a linear search to look for this marker to properly load section data. This isn't so bad because it will only have to be done once per layout call, possibly less. However, linear search sucks, especially when you have to inflate a whole bunch of views to do it.

In just the first item.

slm_sectionStart="true"

or

params.setSectionStart(true);

Pros

  • Trivial to use.

Cons

  • Performance for large sections may be compromised for fresh layouts and scrolling upwards.
  • Have to manage the first item in a section.

1.b Section ids

In addition to the above, add unique section ids to each item, and let SuperSLiM look for the first item. Better than the above because a binary search can be used.

In all section items.

slm_sectionId="56"

or

params.sectionId=56

Pros

  • Simpler to use than the current system.
  • Better performance than 1.a.

Cons

  • Must mark up all items.
  • Still have to manage the first item in a section.

2. Provide section start positions

2.a Out-of-band channel

Provide section start positions in a second channel. This means you would have to implement an interface in your adapter and pass it in the LayoutManager constructor. Also, if you change the adapter on the RecyclerView, LayoutManager gets updated with it automatically so there is no need to manually update it.

interface SectionAdapter {
    List<Integer> getSectionStartPositions();
}

The LayoutManager would pull the data from this just once for each adapter change. It would then keep it up to date according to your notify data change calls.

Pros

  • Don't need to mark up any items.
  • No performance loss.
  • Can switch to this method during runtime with an adapter change.

Cons

  • Still have to manually generate section first positions.

2.b Status quo

The current situation where you have to update and notify for the current item and any later if you are removing or adding items.


Now, I am thinking of implementing all three of these suggested changes. This means you would start with using 1.a or 1.b, and if you have performance problems, consider using 2.a.

If you have any ideas, or an opinion, please comment.

@TonicArtos
Copy link
Owner Author

@PatrickDattilio, and @Eifelschaf. You've been given me plenty of feedback recently so I'd appreciate it if I could have your thoughts. Thanks.

@PatrickDattilio
Copy link

I worry about performance with the 1 solutions. Scroll performance is imperative, I'd personally prefer slightly more intricate setup with absolutely no performance loss. I think 2.a. seems pretty straightforward, it resembles eowise's sticky header setup. Thinking ahead, how might a list of section start positions work with nested sections/headers?

@TonicArtos
Copy link
Owner Author

SLMs already get their data ahead of time from the layout params because the configuration is fetched before the SLM is initialised. So for a nesting SLM you would just package the subsection start positions in the same manner, of course, the nesting SLM would have to track multiple arrays, one for each nest section. Does that make sense?

@PatrickDattilio
Copy link

I'm not sure I follow. If I have Section A, and it contains 50 nested sections/headers (B), who tracks the start positions for each instance of B?

In my SuperSLiMExample after the first two sections (The first being large view, the second being the navigation bar), each view alternates between a header and it's "child" view. Would I simply add each of the header positions to the list in the SectionAdapter interface and setFirstPosition for each header to be the navbar?

@GoodSir42
Copy link

Hi guys,

I also prefer solution 2a as this would save the setup of each and every view every time it's requested for layout. I would really prefer doing this setup only once as the positions of a header should not change if the adapter is not changed (so if the notify method is not called).

@TonicArtos
Copy link
Owner Author

@PatrickDattilio, The subsections have to be declared up front to the SLM, and so are embedded in the first item. The SLM tracks the subsection data. Working with the example you posted before, the changes to adapter would look something like this.

// LayoutManager will pull this data once for each configuration change, and then track changes to it.
public List<Integer> getSectionStartPositions() {
    ArrayList<Integer> startPositions = new ArrayList<>();
    // Add the top level sections only.
    startPositions.add(TOOLBAR_POSITION); // 0
    startPositions.add(NAVIGATION_POSITION); // 1
    return startPositions;
}

@Override
public void onBindViewHolder(RecyclerView.ViewHolder holder, int position) {
    final LayoutManager.LayoutParams lp = (LayoutManager.LayoutParams) holder.itemView.getLayoutParams();
    int type = getItemViewType(position);
    switch (type) {
    // snip
        case NAVIGATION:
            SubSectionSlm.LayoutParams ssLp = new SubSectionSlm.LayoutParams(lp)
            // The SLM will pull this data once for each configuration change, and then track changes to it.
            // Data changes are bubbled down to SLMs from the LayoutManager.
            // Argh, I really have to make sure this is modelled properly.
            ssLp.setSubSectionStartPositions(mNavigationSectionStartPositions);
            ssLp.setSlm(SubSectionSlm.ID);
            holder.itemView.setLayoutParams(ssLp);
            break;
    //snip
}

I still can't decide on a good name for a section which contains other sections. I want something that will be a good descriptor so maybe, LinearContainerSlm. Or maybe, I'll see about making all SLMs support subsections. I think that'll be much easier now that 2.a is how section declaration will work.

@TonicArtos
Copy link
Owner Author

Okay, the decision is to go with 2.a. I've updated the OP with the specific changes you will see.

@TonicArtos TonicArtos changed the title Having to update many items for every addition or removal sucks. Section declaration is subpar Mar 18, 2015
@TonicArtos TonicArtos modified the milestone: 5 Mar 20, 2015
@TonicArtos TonicArtos modified the milestone: 5 Mar 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants