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

Feature/customizable inkwell #65

Merged
merged 3 commits into from Oct 27, 2020
Merged

Feature/customizable inkwell #65

merged 3 commits into from Oct 27, 2020

Conversation

WieFel
Copy link
Collaborator

@WieFel WieFel commented Oct 26, 2020

Makes the InkWell in BackdropNavigationBackLayer customizable:

  • itemPadding: set padding for list items.
  • itemCustomBorder: set custom border for items.
  • itemSplashColor: set custom splash color.

Closes #59.

@WieFel WieFel requested a review from daadu October 26, 2020 14:17
@daadu
Copy link
Member

daadu commented Oct 27, 2020

@WieFel Should we add customBorder as Shape around the item of the list as well?

@WieFel
Copy link
Collaborator Author

WieFel commented Oct 27, 2020

@daadu what do you mean exactly?
I think that is what itemCustomBorder already does...

Copy link
Member

@daadu daadu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this. So that the itemCustomBorder is added to item itself along with InkWell, and we can rename itemCustomBorder to itemShape

lib/navigation.dart Outdated Show resolved Hide resolved
lib/navigation.dart Show resolved Hide resolved
@daadu
Copy link
Member

daadu commented Oct 27, 2020

Rest looks fine to me.

@WieFel
Copy link
Collaborator Author

WieFel commented Oct 27, 2020

There is one problem left:
When I use the following code:

        backLayer: BackdropNavigationBackLayer(
          items: [
            ListTile(title: Text("Widget 1")),
            ListTile(title: Text("Widget 2"), tileColor: Colors.orange,),
          ],
          onTap: (int position) => {setState(() => _currentIndex = position)},
          separatorBuilder: (context, index) => Divider(),
          itemCustomBorder:
          RoundedRectangleBorder(borderRadius: BorderRadius.circular(16)),
        ),

The border is not actually applied to the list item:

Adding clipBehavior: Clip.antiAlias to the Container solves the problem and borders are applied. The question now is if we want to hardcode clipBehavior or if we should expose it as property as well @daadu ?

@daadu
Copy link
Member

daadu commented Oct 27, 2020

Oops! @WieFel This is getting complicated. I would suggest going back to only modifying the customBorder of InkWell. Rename itemCustomBorder to itemSplashBorder.

@WieFel What do you think?

@daadu
Copy link
Member

daadu commented Oct 27, 2020

User can anyways modify item as he wants, the issue only was with InkWell.

@daadu
Copy link
Member

daadu commented Oct 27, 2020

I only suggested setting shape of item because the name (itemCustomBorder) suggested that this parameter is for item. A better name should be itemSplashBorder.

@WieFel WieFel force-pushed the feature/customizable_inkwell branch from 18fc37a to e4c9704 Compare October 27, 2020 13:44
@WieFel
Copy link
Collaborator Author

WieFel commented Oct 27, 2020

@daadu ok no problem.
Reverted the commits. I'll modify the property name and then this is ready for merge...

Copy link
Member

@daadu daadu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@daadu daadu merged commit ebe387d into master Oct 27, 2020
@daadu daadu deleted the feature/customizable_inkwell branch October 27, 2020 14:41
@daadu
Copy link
Member

daadu commented Oct 28, 2020

This feature has landed with v0.4.7 on pub.dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BackdropNavigationBackLayer] Customizable InkWell border
2 participants