MediaSend update 1 : Moved fragment UI to compose #4
Conversation
| ) { | ||
|
|
||
| // span logic: screenWidth / media_picker_folder_width | ||
| val folderWidth = dimensionResource(R.dimen.media_picker_folder_width) |
There was a problem hiding this comment.
You probably want to port this into our compose Dimension.kt instead. You can remove media_picker_folder_width and other related xml dimensions if they are not needed or moved to the kt file instead.
| modifier = Modifier | ||
| .align(Alignment.BottomCenter) | ||
| .fillMaxWidth() | ||
| .height(50.dp) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| @Deprecated |
There was a problem hiding this comment.
One thing to be mindful of if you decide to keep all the old stuff, is to not forget to clean it out later.
With git history you can easily access old files you should be ok deleting and cleaning everything you don't need anymore.
Don't forget to clean out the layout.xml files, and possibly unused views.xml, dimensions, etc...
| .clickable(onClick = onClick) | ||
| ) { | ||
| Box(modifier = Modifier.aspectRatio(1f)) { | ||
| GlideImage( |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| modifier = Modifier.fillMaxSize(), | ||
| contentScale = ContentScale.Crop | ||
| ) | ||
| // Bottom shade overlay |
There was a problem hiding this comment.
Do you think it could be achieved more easily with the innerShadow modifier?
| selected.indexOfFirst { it.uri == media.uri } | ||
| } | ||
|
|
||
| // Matches adapter rules: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| .padding(end = 2.dp, bottom = 2.dp) | ||
| .aspectRatio(1f) | ||
| .combinedClickable( | ||
| onClick = { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| Box( | ||
| modifier = modifier | ||
| .padding(end = 2.dp, bottom = 2.dp) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| ) | ||
|
|
||
| // Border overlay (replaces @drawable/mediapicker_item_border_dark View) | ||
| Box( |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| painter = painterResource(R.drawable.triangle_right), | ||
| contentDescription = null, | ||
| modifier = Modifier | ||
| .size(width = 15.dp, height = 18.dp) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| val context = LocalContext.current | ||
| val activity = context as? FragmentActivity | ||
| val showManage = remember(activity) { |
There was a problem hiding this comment.
Suspicious of logic managed in a composable. It should get its state handed to it.
This is tied to a specific activity. I wonder how this is going to work once you go to full compose.
AttachmentManager might no longer be the right file for this logic? We need to rethink the format now that we are moving things to a modern structure. Should this be VM logic?
| .fillMaxSize() | ||
| .background(LocalColors.current.background) | ||
| ) { | ||
| items(folders) { folder -> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| val mediaItemGridSpacing : Dp = 2.dp, | ||
| val mediaPlayOverlay : Dp = 36.dp, | ||
|
|
||
| val smallRadius : Dp = 26.dp |
There was a problem hiding this comment.
This name doesn't seem to match what it does. Usually radius are for rounded corners.
Maybe mediaItemIndicatorSize ?
| .padding(LocalDimensions.current.xxsSpacing), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| IndicatorOn(size = LocalDimensions.current.smallRadius) |
There was a problem hiding this comment.
It looks like IndicatorOn and IndicatorOff always use the same size. Maybe you can leave the size as an attribute to customise them but instead of passing the value in this case you could give them a default value directly as the common usage.
| import androidx.compose.ui.tooling.preview.Preview | ||
| import androidx.compose.ui.unit.Dp | ||
| import com.bumptech.glide.integration.compose.ExperimentalGlideComposeApi | ||
| import com.bumptech.glide.integration.compose.GlideImage |
There was a problem hiding this comment.
Make sure Glide's references are removed so we can remove the library eventually
| .aspectRatio(1f) | ||
| .border( | ||
| width = LocalDimensions.current.borderStroke, | ||
| color = Color.White.copy(alpha = 0.20f) |
There was a problem hiding this comment.
A new color is suspicious here. Shouldn't it be our LocalColors.current.borders color instead?
Moved the old Fragment XMLs to Compose.
New ComposeFragments are created to replace the old fragments for the MediaSendActivity.
Part 2 in progress : MediaSendFragment UI to compose.