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

StructForm widget for creating modals with a Form #57

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

aodhan-domhnaill
Copy link

Example use case,

type Transaction struct {
	gorm.Model
	Timestamp            time.Time `fyne:"field"`
	Amount               float64   `fyne:"field"`
	Description          string    `fyne:"field"`
}

type FinancialData struct {
	db *gorm.DB
}

func (fd *FinancialData) AddTransaction(tr *Transaction) error {
	if err := fd.db.Create(&tr).Error; err != nil {
		return err
	}
	return nil
}

func main() {
	// Open a database connection using the sqlite driver
	db, err := gorm.Open(sqlite.Open("accounts.db"), &gorm.Config{})
	if err != nil {
		panic("Failed to connect to database!")
	}

	fd := data.NewFinancialData(db)

	....

	transactionModal = widget.NewStructForm(
		myWindow.Canvas(),
		data.Transaction{},
		func(s interface{}) {
			fd.AddTransaction(s.(*data.Transaction))
		},
	)

	....
}

@aodhan-domhnaill aodhan-domhnaill changed the title Adding struct form StructForm widget for creating modals with a Form Apr 18, 2023
@Bluebugs
Copy link
Contributor

This is pretty cool. Just an idea here, what do you think of changing field to actually be the widget type to use. For example, you could have:

type Transaction struct {
	Timestamp            time.Time `fyne:"entry"`
	Amount               float64   `fyne:"slider:0-100"`
	Description          string    `fyne:"entry"`
        Paid                 bool      `fyne:"checkbox"`
}

I will totally accept the PR with just handling the entry type.

@aodhan-domhnaill
Copy link
Author

That's a good idea. I'll work on a change.

@andydotxyz
Copy link
Member

Isn't this what dialog.NewForm is for?

Copy link
Contributor

@Bluebugs Bluebugs left a comment

Choose a reason for hiding this comment

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

Can you update the top README file and add an example in cmd?

"fyne.io/fyne/v2/widget"
)

type StructForm struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add some documentation for all exported type?

widget.BaseWidget
structType reflect.Type
modal *widget.PopUp
canvas fyne.Canvas
Copy link
Contributor

Choose a reason for hiding this comment

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

You do not need to store this value if you only access it from within NewStructForm.

submit func(s interface{})
}

func NewStructForm(c fyne.Canvas, s interface{}, submit func(s interface{})) *StructForm {
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be useful to split the function in two, one to create just the form and another to pack it in a dialog.

@Bluebugs
Copy link
Contributor

Isn't this what dialog.NewForm is for?

This generate the item of NewForm by using reflection. It could use NewForm internally I guess.

@andydotxyz
Copy link
Member

I think this is confusing widgets and modal dialogs. A widget can be used in a modal dialog - or if you want to make it a modal popup then use dialog for a consistent look.

From my cursory glance it seems like this is a powerful widget that should be separated from the modality concept - so it can be used for any location.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

This is looking good, but note you have outstanding comments from @Bluebugs.

Just thinking out loud here, but did you consider extending Form instead of BaseWidget? Perhaps the NeeStructForm could even extend form - I'm not sure but I wonder if that is cleaner? (I think the CreateRenderer could directly return Form.CreateRenderer).

@andydotxyz
Copy link
Member

Just trying to catch up on things, where did this get to @Bluebugs @aodhan-domhnaill ? It's such a cool feature, I am embarrassed that we let it sit.

@aodhan-domhnaill
Copy link
Author

Just trying to catch up on things, where did this get to @Bluebugs @aodhan-domhnaill ? It's such a cool feature, I am embarrassed that we let it sit.

Sorry. I kind of forgot about this because life stuff. I can try to pick it up again soon

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