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

Bug in Range() implementation? #825

Closed
rtrzebinski opened this issue Apr 2, 2023 · 9 comments
Closed

Bug in Range() implementation? #825

rtrzebinski opened this issue Apr 2, 2023 · 9 comments

Comments

@rtrzebinski
Copy link

rtrzebinski commented Apr 2, 2023

Description:

I'm having a issue with the following code:

app.Range(h.rows).Slice(func(i int) app.UI {
  return &h.rows[i]
}),

It works fine when the component is loaded for the first time, but not when the slice and range needs to be updated.

On rebuilding the underlying slice with data from API call, the UI behaves incorrectly.

I tried manually running component.Update - did not help.

The only work around is reloading the page with ctx.Reload() but it's doing an ugly page refresh, and it's not a solution really.

Steps to reproduce:

git clone [email protected]:rtrzebinski/simple-memorizer-4.git
git checkout 58475e1c1171863229ff0de07d7f9dbb4c2c8bd4
make dev
make run
open http://localhost:8000/exercises

Add exercises: (question / answer):

11 / 11
22 / 22
33 / 33
44 / 44

(in order to retry with a fresh database simply run make dev && make run again and refresh the page)

Expected result:

Exercises will appear on the list as they are added.

No manual page reload is needed to display them properly.

Actual result:

Only 11 / 11 (first one added) keeps being duplicated, regardless of the following inputs:

Screen Shot 2023-04-02 at 06 59 29

Where is the code?

Range -> https://github.com/rtrzebinski/simple-memorizer-4/blob/58475e1c1171863229ff0de07d7f9dbb4c2c8bd4/internal/frontend/components/exercises.go#L40

Update trigger -> https://github.com/rtrzebinski/simple-memorizer-4/blob/58475e1c1171863229ff0de07d7f9dbb4c2c8bd4/internal/frontend/components/exercises.go#L71

Actual UI update -> https://github.com/rtrzebinski/simple-memorizer-4/blob/58475e1c1171863229ff0de07d7f9dbb4c2c8bd4/internal/frontend/components/exercises.go#L89

@maxence-charriere
Copy link
Owner

Can you provide the ExerciseRow component code too ?

@rtrzebinski
Copy link
Author

Of course:

type ExerciseRow struct {
	app.Compo

	exercise models.Exercise
}

// The Render method is where the component appearance is defined.
func (h *ExerciseRow) Render() app.UI {
	return app.Tr().Style("border", "1px solid black").Body(
		app.Td().Style("border", "1px solid black").Body(
			app.Text(h.exercise.Question),
		),
		app.Td().Style("border", "1px solid black").Body(
			app.Text(h.exercise.Answer),
		),
	)
}

https://github.com/rtrzebinski/simple-memorizer-4/blob/58475e1c1171863229ff0de07d7f9dbb4c2c8bd4/internal/frontend/components/exercise_row.go

@maxence-charriere

@oderwat
Copy link
Contributor

oderwat commented Apr 2, 2023

Please try:

app.Range(h.rows).Slice(func(i int) app.UI {
  return h.rows[i].Render()
}),

@maxence-charriere I think this is related to the problem from #767, which is still not fixed (even if it needs another way to fix it than his method).

@rtrzebinski
Copy link
Author

@oderwat This helped thank you so much! It would be good to document that :)

@oderwat
Copy link
Contributor

oderwat commented Apr 2, 2023

@rtrzebinski please open the issue again. I am not positive, that this is a "good" way to solve it and has no unwanted side effects. I think when doing it like this you skip the "OnMount" on those components and that is not really how it should work.

Likewise, I wanted @maxence-charriere to chime in on that for some time already. I think there has definitely something to be changed.

@maxence-charriere
Copy link
Owner

maxence-charriere commented Apr 2, 2023

Try to export the exercise field in the ExerciseRow component. I think the problem here is that there is nothing to compare to trigger a diff.

exercise => Exercise

@rtrzebinski
Copy link
Author

Re-opening the issue as requested by @oderwat

@maxence-charriere I tried that - making ExerciseRow.Exercise exported (capital letter) indeed improved adding new rows, however without the Render() in:

				app.Range(h.rows).Slice(func(i int) app.UI {
					if h.rows[i] != nil {
						return h.rows[i].Render()
					}
					return nil
				}),

It was messing up indexed making deleting and display messed up again, so I'll stick with the Render() solution as it works well.

Please let me know if you need some more examples / steps to reproduce, I am happy to provide these.

@rtrzebinski rtrzebinski reopened this Apr 3, 2023
@gepengscu
Copy link

gepengscu commented Jun 13, 2023

app.Range(h.rows).Slice(func(i int) app.UI {
  return h.rows[i].Render()
}),

I avoid the "Bug" in the same way.
Never save ui in slice.
Just save nessary data in slice and render ui with data in slice.

type tabs struct{
  app.Compo
  items []*Item
}

type Item struct{
  Key string
  Label string
}
func (this *tabs) Render() app.UI {  
  return app.Div().
      Body(
        app.Range(this.items).Slice(
	   func(i int) app.UI {
	      return this.renderTab(this.items[i])
	   }))
}

func (this *tabs) renderTab(item *Item) app.UI {  
  return app.Text(item.Label).
    OnClick(this.onCloseTab(item), item.Key)
}

func (this *tabs) onCloseTab(item *Item) app.EventHandler {
  return func(ctx app.Context, e app.Event) {  
    this.items = removeItemInSlice(this.items, item)
  }
}

@oderwat
Copy link
Contributor

oderwat commented Jan 4, 2024

Never save ui in slice.

This would mean you can't use a component in a slice and this would mean that there is something seriously missing in Go-App. What do you think @maxence-charriere

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

No branches or pull requests

4 participants