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

add gap extension #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lukepighetti
Copy link

@lukepighetti lukepighetti commented Nov 9, 2023

This adds a gap extension to make it easier to evenly space Rows and Columns

return Row(
  children: [
    Icon(Icons.abc),
    Icon(Icons.air),
    Icon(Icons.add),
  ].gap(10),
);

@letsar
Copy link
Owner

letsar commented Nov 10, 2023

Can you export it through a specific library called gap_extensions so that the developers can decide whether they want to import it?

@lukepighetti
Copy link
Author

lukepighetti commented Nov 10, 2023

I gave the extension a package unique name so there wouldn't be any chance of collisions on the extension name itself. The method name and type extended combination is unique enough that I doubt there will be issues there. Discoverability and ease of use will be low if people have to add a second import line to use it. In the case of any collisions it can be ignored in the import.

@letsar
Copy link
Owner

letsar commented Nov 10, 2023

I'm was not worried about name collisions but more about unwanted extensions popping in intellisense. But since VS Code provide them anyway through intellisense whether the library is imported or not, it does not matter.

I'm not really convinced having more extensions would make Flutter code more readable in general, but if this PR gets a lot of traction (let's say at least 50 👍), I'll merge it and I'll add a sliverGap extension to complete this feature.

@LowLevelSubmarine
Copy link

LowLevelSubmarine commented Dec 18, 2023

I've implemented a similiar approach in my projects recently. But one additional thing I did, was to only add gap via the extension if there wasn't already a gap widget defined by the user in the list. This allows developers to adjust the spacing at a specific index without having to think about subtracting the spacing the addGap() extension adds. It also prevents cluttering the widget tree with multiple Gap Widgets behind each other.

I would also argue, that adding this extension completes this library in a way, as developers currently dont have a simple go-to approach for adding overall spacing to multiple widgets although it is a (i guess) widely spread use-case. I'd also say that adding a seperate import is the way to go.

@LowLevelSubmarine
Copy link

If you dont feel like adding this functionality to this package itself, maybe creating another gap-official companion-package is acceptable?

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.

3 participants