-
Notifications
You must be signed in to change notification settings - Fork 194
Labs screen #4573
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
base: develop
Are you sure you want to change the base?
Labs screen #4573
Conversation
8797a53
to
efdc968
Compare
@amshakal in Figma i noticed that the icon has a bg subtle primary color as a background, however the type is default solid, which uses bg subtle secondary as a background color. So for now I am using the default (non solid) type here since otherwise the background would be the same as the background of the view itself. We do not have in compound a type that uses subtle primary as background for these big icons, should we make a new one then? |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #4573 +/- ##
===========================================
+ Coverage 78.26% 78.93% +0.66%
===========================================
Files 862 863 +1
Lines 81188 81172 -16
===========================================
+ Hits 63545 64075 +530
+ Misses 17643 17097 -546
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Generated by 🚫 Danger Swift against efdc968 |
I changed the designs based on your feedback :) |
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.
LGTM, although one comment about the acceptance criteria.
ListRow(label: .default(title: L10n.screenAdvancedSettingsLabs, | ||
icon: \.labs), | ||
kind: .navigationLink { | ||
context.send(viewAction: .labs) | ||
}) |
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 think based on the meta issue we want this button to automatically hide if there aren't any settings in LabsOptionsProtocol
.
fixes #4561
This PR adds the labs screen in the settings.