Skip to content

Conversation

@CaseyHillers
Copy link
Contributor

@CaseyHillers CaseyHillers commented Mar 20, 2020

This adds a dropdown button to the build dashboard appbar that allows for switching the current branch. It adds updates FlutterBuildState to be able to change the branch that data is shown from.

It currently erases previous branch data when switching to a new branch.

Open Questions

I feel moreStatusesExist can have a better name. I am looking for a variable name that says the build state knows that more commit statuses exist from the backend, and they have to be loaded over the network. In certain cases (such as branches not master), there is only a few commits and Cocoon should not keep trying the infinite load approach

Issues

flutter/flutter#51807 - Flutter branching for releases

Tests

Tests for update branch and new behavior.

Widget tests to check the dropdown exists

Preview

https://testchillers-dot-flutter-dashboard.appspot.com/#/build

@CaseyHillers
Copy link
Contributor Author

/cc @Hixie if you have time, can you take a look at this? It's for adding a dropdown to the build dashboard to change branches.

I'm running into an issue where I can't get the text color to be white when the dropdown is idle, and have it be black when the dropdown menu is open.

@CaseyHillers CaseyHillers changed the title Change branch dropdown menu on build dashboard Branch dropdown menu on build dashboard Mar 20, 2020
value: buildState.currentBranch,
icon: Icon(
Icons.arrow_downward,
color: Colors.white,
Copy link
Contributor

Choose a reason for hiding this comment

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

in theory the color should be picked up automatically by the ambient theme, something like how i made it happen for the SIGN IN button. cc @HansMuller for advice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I am going to submit on green to enable testing of the branching release strategy. Will follow up with smoothing this out next week.

child: Text(value, style: TextStyle(color: Colors.black)),
);
}).toList()),
Container(width: 10), // Padding between branches and sign in
Copy link
Contributor

Choose a reason for hiding this comment

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

SizedBox(width: 10) is more efficient

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Added.

@Hixie
Copy link
Contributor

Hixie commented Mar 20, 2020

I'm still learning the codebase, but LGTM.

@CaseyHillers CaseyHillers merged commit c5fed02 into flutter:master Mar 20, 2020
@CaseyHillers CaseyHillers deleted the update_branch branch March 20, 2020 21:33
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.

2 participants