-
-
Notifications
You must be signed in to change notification settings - Fork 310
Fix clock when board is flipped #1437
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
Fix clock when board is flipped #1437
Conversation
|
I'm not sure this fix would work better. You need to store an unique key in a |
312b12a to
b3fed1a
Compare
|
|
||
| /// The side the board is displayed from. | ||
| /// | ||
| /// It should only be used when you need the state of the board header or footer to be preserved. |
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.
This comment is misleading. You're not preserving the state, but you're forcing the destruction of the widgets (and recreation) when the ValueKey changes.
Since we always have that information available, I'd make that parameter mandatory. It would help to avoid further surprises.
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.
If I understand correctly the documentation it will not recreate the widget if what you mean by widget is the underlying element. If I had kept the keys in the CountdownBuilder I agree that the underlying element would have been recreated which is why I moved it in the AnalysisLayout. I believe that thanks to the local key, it will simply call update on the element to update the widget configuration.
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.
Since we always have that information available, I'd make that parameter mandatory. It would help to avoid further surprises.
I didn't want to force this parameter as it is only useful when there is a board footer or header. Should I still make it mandatory and keep only the first line of the doc comment to not make it confusing ?
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 got it. But there is no harm making it mandatory as we have the data ready and it's not used in a lot of places. Also it is future proof, in case we'll use stateful widgets in other screens than broadcast.
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 made the pov parameter mandatory and added comments where we use a key to explain why it is needed
the board is flipped was incorrect, this commit corrects it.
b3fed1a to
6d1263a
Compare
Close #1408
The fix done in the PR #1428 could not work. This time it should really be fixed.