-
Notifications
You must be signed in to change notification settings - Fork 338
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Revert "Make the layout more adaptive with Flexbox when using sp"
This reverts commit c7d37a9.
- Loading branch information
1 parent
2043da7
commit 86a64e8
Showing
4 changed files
with
49 additions
and
73 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HI @FridoDeluxe @igor-cali @fAntel, a couple of weeks back a commit introduced a bug to Forecastie which is fairly serious, breaking the UI. Before I noticed, I accepted a series of other PRs. I've tried to revert the original problem, but may have made it worse. Would you be able to help? I ask you as you've made most significant additions to the app over the last few year.
Thanks, robin
86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the referenced bug?
86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#618 , also
#628
86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've since done a
git reset --hard
, back to v1.18. This may have been a bad idea. I then released v1.20, which is v1.18 with the replacement API key.86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, I'm neither an expert of GitHub nor Git itself.
It seems like the revert didn't work. For example in
activity_scrolling.xml
the master branch still shows the changes introduced in c7d37a9.86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FridoDeluxe Yes, so it does. I don't understand that, I'm sure I did a hard reset back to the end of October.....
86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I think I understand what happened. Reintroducing the "chance of precipitation" PR brought all the other changes which had happened between the hard reset point and that merge. I did another hard reset, then added the new API key again. It appears I'm not a github/git expert either.
I think the 1.20 release out shortly on f-droid will be OK.
86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an aside, I've told Tomas I won't be looking after the app any more, maybe he will take it on himself again, or maybe get somebody else to do it.
86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sad to hear, but thank you for all the years of organizing and advancing the Forecastie development!
86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ciao @robinpaulson , are there any Issue or PR left to complete for the UI? Please let me know if help is needed and where.
86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @igor-cali . I'm not sure, I had abandoned the flexbox UI improvements, given the original coder is not responding. If you want to take up where he left it though, that would be great.
As far as I can tell, all the problems from the bad commit have been fixed. In the next day or so, I'll merge the PR you made, plus a couple of others put forward in the last few days.
86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robinpaulson I'm not familiar with Flexbox; never had a look at it. Do you think it's worth the effort to reintroduce that change, after all the struggle to revert and revamp? I could try to have a look, if you like. IMHO, I would prefer to improve the native code rather than adding an extra library.
86a64e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@igor-cali I'm not sure the Flexbox code is worth it (I think I misunderstood your original question), there are other more pressing issues. Up to you I guess.