Skip to content
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

Add graph background color argument #5

Open
daniel-eh opened this issue Feb 2, 2022 · 2 comments
Open

Add graph background color argument #5

daniel-eh opened this issue Feb 2, 2022 · 2 comments
Assignees

Comments

@daniel-eh
Copy link

The background color for the canvas is hard coded to MaterialTheme.colors.surface

Just add an optional argument to the LineGraph composable for the graph background color, then set the bgColor variable in the composable to be equal to this argument. You can default it to the surface color if you wish and make it optional.

@thsaravana thsaravana self-assigned this Feb 14, 2022
@thsaravana
Copy link
Member

True, the background color of LineGraph is set to MaterialTheme.colors.surface. But it is by no means hard coded. MaterialTheme.colors.surface picks up the surface color set in the app's theme. In the sample app, you will notice that this is Grey50 mentioned in PlotTheme. You can override the color there. Or if you prefer to handle this as an isolated case, then you can always use ThemeOverlays like how it is done in LineGraph4.kt. This keeps the API clean and sticks to the principle of Theme driving the color, shape and typography.

@JesusM
Copy link

JesusM commented Mar 7, 2022

True, the background color of LineGraph is set to MaterialTheme.colors.surface. But it is by no means hard coded. MaterialTheme.colors.surface picks up the surface color set in the app's theme. In the sample app, you will notice that this is Grey50 mentioned in PlotTheme. You can override the color there. Or if you prefer to handle this as an isolated case, then you can always use ThemeOverlays like how it is done in LineGraph4.kt. This keeps the API clean and sticks to the principle of Theme driving the color, shape and typography.

Wether I agree with keeping the library with a clean API, it's also true that most of material-related compose components provide some sort of personalisation. Taking LineGraph component as an example, I guess you could keep a clean API and allow personalisation moving the bgColor to a parameter with a default value based on material theme, something like this:

@Composable
fun LineGraph(
    plot: LinePlot,
    modifier: Modifier = Modifier,
    bgColor: Color = MaterialTheme.colors.surface,
    onSelectionStart: () -> Unit = {},
    onSelectionEnd: () -> Unit = {},
    onSelection: ((Float, List<DataPoint>) -> Unit)? = null
)

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

No branches or pull requests

3 participants