search

fyne-io / 7guis

Fyne implementation of 7GUIs: A GUI Programming Benchmark - https://eugenkiss.github.io/7guis/
15 stars 7 forks source link

Add the initial code for the temperature converter #1

Closed Jacalz closed 4 years ago

Jacalz commented 4 years ago

It is a bit ugly with a hack to workaround entry.SetText() triggering the entry.OnChanged() function along with ugly string, int and float conversion. Anyhow, it works and looks good rendered :)

Jacalz commented 4 years ago

FYI, this is my first open source code contribution ever :champagne:

andydotxyz commented 4 years ago

Congratulations on your first ever contribution, this was really close. A few tweaks and we could land it I'm sure :)

Jacalz commented 4 years ago

I have updated the code according to the code review. I haven't had the time to test it yet, but I will do that later today.

andydotxyz commented 4 years ago

Thanks for updating - but please don’t push code you haven’t tested

Jacalz commented 4 years ago

Yeah you are right. It was stupid, I really don't know what I thought at the time :slightly_frowning_face:

andydotxyz commented 4 years ago

Thanks for this. I don't think we can merge it until the text updating is fixed. To get it in sooner maybe a similar workaround to before is required?

Jacalz commented 4 years ago

Yeah I think you are right. I can add back the workaround and then submit a new PR when the issue is fixed upstream.

Jacalz commented 4 years ago

The workaround has been applied again until the bug gets fixed upstream :+1:

Jacalz commented 4 years ago

I think this should suffice now :)

Jacalz commented 4 years ago

Thanks for merging. All that is missing now is an image in the README.md file :)

andydotxyz commented 4 years ago

You're welcome to PR that as well ;)

Jacalz commented 4 years ago

There is a reason that I never included a picture, it would not be consistent with the other picture in the readme 😉

andydotxyz commented 4 years ago

done