-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Converting Nimbus Branches and Nimbus Fragments to Compose. #27640
Conversation
No Taskcluster jobs started for this pull requestThe `allowPullRequests` configuration for this repository (in `.taskcluster.yml` on the
default branch) does not allow starting tasks for this pull request. |
Hey @vrrashkov, this looks like a very good first pass at converting the Nimbus view into Compose. Thank you for the contribution. I have various review feedback that ranges from formatting, naming, etc that would help align the code closer to what we would expect from each other in the team. I am wondering if you would prefer to receive such feedback and address them. I know this could be cumbersome depending on your other commitments. So, I can either offer the feedback if you prefer otherwise I could also help make the adjustments to get your work through the finish line. Thanks! |
@gabrielluong Yes sounds good. I do plan to do more contributions in the future, so I am open to feedback. |
This pull request has conflicts when rebasing. Could you fix it @vrrashkov? 🙏 |
Based on this issue #19534 and I believe this might also be the same one #27632. It was design related but instead of fixing only the color I migrated the code to compose for Nimbus Branches and Nimbus Experiments.
Experiments:
Branches:
The logic is kept the same, there are no changes to the BranchesStore/BranchesController. That's why there are no new tests and the old ones are working.
This mozilla.components.service.nimbus this module is no longer used. The new elements for the UI are all inside the BranchesFragment and ExperimentsFragment. I was not sure if I should put them in the mozilla.components.service.nimbus module, so I checked other places and saw that "WallpaperSettings" for example is using compose element directly in the Fragment and not in a separate module so I did the same.