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

Horizontally stacked flexbox and more #10

Merged
merged 6 commits into from
Apr 9, 2023
Merged

Conversation

jon4hz
Copy link
Contributor

@jon4hz jon4hz commented Jan 30, 2023

Hi,

Initially I just wanted to add support for a horizontally stacked flexbox but somehow I ended up doing a lot more changes to fit my needs. Basically all of them are breaking.

I changed the following things:

  • create dedicated package for the table -> not all flexbox users will need the table, so it shouldn't be included by default
  • Implement use variadic function to add cells #9
  • move flexbox to subfolder
  • Renamed a lot of stuff for easier readability. E.g. stickers.FlexBoxRow -> flexbox.Row

Please feel free to adopt whatever you like, maybe for v2 of this library, since it's very breaking.

@76creates
Copy link
Owner

Hey there, thanks for investing time into stickers, its awesome to see it being used to such extent.
Theres a lot of changes for sure, Ill need bit more time to review them, will follow up soon ⚡

// SetMinWidth sets the cells minimum width, this will not disable responsivness
func (r *FlexBoxCell) SetMinWidth(value int) *FlexBoxCell {
// SetMinWidth sets the cells minimum width, this will not disable responsivness.
// This has only an effect to cells of a normal FlexBox, not a HorizontalFlexBox.
Copy link
Owner

Choose a reason for hiding this comment

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

normal FlexBox > FlexBox

@@ -66,8 +67,9 @@ func (r *FlexBoxRow) GetCellCopy(index int) *FlexBoxCell {

// GetCellWithID returns the cell with the given ID if existing
// note: forces the recalculation if found
// returns nil if not found
func (r *FlexBoxRow) GetCellWithID(id string) *FlexBoxCell {
//
Copy link
Owner

Choose a reason for hiding this comment

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

style: extra row

Copy link
Owner

Choose a reason for hiding this comment

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

Feels like HorizontalFlexBox is basically mirror of the FlexBox, this is not really ideal as it requires user to make a change on two essentially same functions, it might be better to create base struct that FlexBox and HorizontalFlexBox would inherit and build upon.
I would leave this increment for the future PR to avoid this PR being overly complex.

Copy link
Owner

Choose a reason for hiding this comment

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

same for column.go

@76creates
Copy link
Owner

76creates commented Apr 9, 2023

Sorry for the massive lag, time became the rarest commodity for the past few months.

[update]
This PR is essentially good, I am not a massive fan of repeating code, but I think it will be ok to merge this PR and then refactor it with another one when I have time to think it trough.

@76creates 76creates merged commit f41dd6b into 76creates:master Apr 9, 2023
76creates added a commit that referenced this pull request Apr 10, 2023
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.

2 participants