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/city input #22

Merged
merged 26 commits into from
May 30, 2022
Merged

Feature/city input #22

merged 26 commits into from
May 30, 2022

Conversation

itsjustdel
Copy link
Contributor

@itsjustdel itsjustdel commented May 27, 2022

CompletionEntry.go copied in to repo. Function at ln 136 "newNavigableList" is the only changes to the file

Options showing well
image

Saving Correctly
image

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.

Just a few change I think are needed.

I can also imagine we could make walking cities.Cities a lot faster if necessary, but at this point let's do that only if it doesn't work fast enough on any of our devices.

home.go Outdated
@@ -32,5 +114,6 @@ func (n *nomad) makeHome() fyne.CanvasObject {
}
cells = append(cells, n.makeAddCell())

return container.New(&nomadLayout{}, cells...)
homeContainer = container.New(&nomadLayout{}, cells...)
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems the reason for this to become a global is because n.autoCompleteEntry() called from n.makeAddCell() doesn't see it. It should be possible to reorder the code. Pass the container to makeAddCell which would pass it further down to n.autoCompleteEntry(). This would avoid the need for a global.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm having trouble with this, if I declare the homeContainer variable inside makeHome(), I get a pointer error when saving the city. If I make a variable in the nomad struct in main.go, and assign, I get no errors but the city doesn't save correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you move the variable declaration into the makeHome then pas s that through makeAddCell into autoCompleteEntry it should work, I think this is what @Bluebugs meant.
Also you could move the declaration to the nomad struct as you say, which may be useful for other code (not sure exactly) it should work as well. The pointer error you mentioned may be because you had two variables of the same name and assigned the wrong one?

location.go Outdated
city := widget.NewRichTextFromMarkdown("# " + l.location.name)
location := widget.NewRichTextFromMarkdown("## " + l.location.country + " · " + l.location.localTime.Format("MST"))
city := widget.NewRichTextFromMarkdown("# " + strings.ToUpper(l.location.name))
location := canvas.NewText(" "+strings.ToUpper(l.location.country)+" · "+l.location.localTime.Format("MST"), color.NRGBA{0xFF, 0xFF, 0xFF, 0xBF})
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to use a named constante for the color? Ideally something that reflect the meaning of its use.

Copy link
Contributor Author

@itsjustdel itsjustdel May 30, 2022

Choose a reason for hiding this comment

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

I'm unable to declare a constant?
(color.NRGBA literal) (value of type "image/color".NRGBA) is not constant

I created a var but unsure where to place it :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, const is very strict for Go but a var at the top level (global is OK here as it's pretending to be a const) make a var( block at the top of the file if one does not exist already.

home.go Outdated
entry := NewCompletionEntry([]string{})
entry.SetPlaceHolder("Add a Place")

cardTexts := []string{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is cardTexts created here? I am not sure you need to hold a reference to it outside of when there is a change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, perhaps a shadow from an earlier solution, I've moved it inside the function

home.go Outdated
Comment on lines 79 to 80
fmt.Println("Bad submit")
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

That should be using fyne.LogError with a custom error.

@itsjustdel itsjustdel merged commit 9303584 into main May 30, 2022
@itsjustdel itsjustdel deleted the feature/city-input branch May 30, 2022 16:40
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.

None yet

3 participants