From 55b6ab129d5a93f785962023fb17277857d853f8 Mon Sep 17 00:00:00 2001 From: Soumen Pal Date: Fri, 7 Mar 2025 23:02:09 +0530 Subject: [PATCH] Fixed Linted issuse related to Magic numbers and long methods and did the requested changes --- .../kiwixmobile/core/help/HelpAdapter.kt | 74 ------- .../kiwix/kiwixmobile/core/help/HelpScreen.kt | 185 +++++++++++------- .../kiwixmobile/core/help/HelpScreenItem.kt | 135 ++++++++----- .../core/help/HelpScreenItemDataClass.kt | 2 +- .../custom/help/CustomHelpFragment.kt | 30 ++- lintConfig.xml | 1 + 6 files changed, 231 insertions(+), 196 deletions(-) delete mode 100644 core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpAdapter.kt diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpAdapter.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpAdapter.kt deleted file mode 100644 index 0051deff0..000000000 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpAdapter.kt +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Kiwix Android - * Copyright (c) 2019 Kiwix - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - * - */ -package org.kiwix.kiwixmobile.core.help - -import android.animation.ObjectAnimator -import android.text.method.LinkMovementMethod -import android.view.LayoutInflater -import android.view.ViewGroup -import androidx.core.view.isGone -import androidx.recyclerview.widget.RecyclerView -import org.kiwix.kiwixmobile.core.base.adapter.BaseViewHolder -import org.kiwix.kiwixmobile.core.databinding.ItemHelpBinding -import org.kiwix.kiwixmobile.core.utils.AnimationUtils.collapse -import org.kiwix.kiwixmobile.core.utils.AnimationUtils.expand - -internal class HelpAdapter(titleDescriptionMap: Map) : - RecyclerView.Adapter() { - private var helpItems = titleDescriptionMap.map { (key, value) -> HelpItem(key, value) } - - override fun onCreateViewHolder( - parent: ViewGroup, - viewType: Int - ): Item = Item(ItemHelpBinding.inflate(LayoutInflater.from(parent.context), parent, false)) - - override fun onBindViewHolder( - holder: Item, - position: Int - ) { - holder.bind(helpItems[position]) - } - - override fun getItemCount(): Int = helpItems.size - - internal inner class Item(private val itemHelpBinding: ItemHelpBinding) : - BaseViewHolder(itemHelpBinding.root) { - @SuppressWarnings("MagicNumber") - fun toggleDescriptionVisibility() { - if (itemHelpBinding.itemHelpDescription.isGone) { - ObjectAnimator.ofFloat(itemHelpBinding.itemHelpToggleExpand, "rotation", 0f, 180f).start() - itemHelpBinding.itemHelpDescription.expand() - } else { - ObjectAnimator.ofFloat(itemHelpBinding.itemHelpToggleExpand, "rotation", 180f, 360f).start() - itemHelpBinding.itemHelpDescription.collapse() - } - } - - override fun bind(item: HelpItem) { - itemHelpBinding.itemHelpTitle.setOnClickListener { toggleDescriptionVisibility() } - itemHelpBinding.itemHelpToggleExpand.setOnClickListener { toggleDescriptionVisibility() } - itemHelpBinding.itemHelpDescription.apply { - text = item.description - movementMethod = LinkMovementMethod.getInstance() - } - itemHelpBinding.itemHelpTitle.text = item.title - } - } -} - -class HelpItem(val title: String, val description: String) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreen.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreen.kt index 2cba40c59..5efb97b49 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreen.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreen.kt @@ -25,14 +25,16 @@ import androidx.compose.foundation.isSystemInDarkTheme import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxHeight import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.ArrowBack +import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Icon @@ -47,10 +49,12 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.colorResource +import androidx.compose.ui.res.dimensionResource import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource -import androidx.compose.ui.unit.dp +import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.sp import androidx.navigation.NavController @@ -58,95 +62,144 @@ import org.kiwix.kiwixmobile.core.R import org.kiwix.kiwixmobile.core.error.DiagnosticReportActivity import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.start -@OptIn(ExperimentalMaterial3Api::class) +val SendDiagnosticReportFontSize = 18.sp + @Composable fun HelpScreen( modifier: Modifier = Modifier, data: List, navController: NavController ) { - val context = LocalContext.current - val isDarkTheme = isSystemInDarkTheme() - val backgroundColor = if (isDarkTheme) colorResource(id = R.color.mine_shaft_gray900) else Color.White val dividerColor = - if (isDarkTheme) colorResource(id = R.color.mine_shaft_gray600) else colorResource( - id = R.color.mine_shaft_gray350 - ) + if (isDarkTheme) colorResource(id = R.color.mine_shaft_gray600) + else colorResource(id = R.color.mine_shaft_gray350) Scaffold( modifier = Modifier.fillMaxSize(), topBar = { - TopAppBar( - title = { - Text( - modifier = modifier.padding(start = 16.dp), - text = stringResource(id = R.string.menu_help), - color = Color.White // Set title text color to white - ) - }, - navigationIcon = { - IconButton(onClick = navController::popBackStack) { - Icon( - imageVector = Icons.Filled.ArrowBack, - contentDescription = "Back", - tint = Color.White // Set navigation icon color to white - ) - } - }, - colors = TopAppBarDefaults.topAppBarColors( - containerColor = Color.Black // Set top app bar background color to black - ) - ) + Row( + modifier = Modifier.fillMaxWidth(), + verticalAlignment = Alignment.CenterVertically + ) { + HelpTopAppBar(navController) + } }, containerColor = backgroundColor - ) { + ) { innerPadding -> + HelpContent(data, dividerColor, innerPadding) + } +} - Column( - modifier = Modifier - .padding(it) - ) { +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun HelpTopAppBar(navController: NavController) { + // Retrieve the actionBarSize from the current theme + val context = LocalContext.current + val actionBarHeight = with(LocalDensity.current) { + // Obtain the height defined in the theme (usually 56dp on phones) + val styledAttributes = + context.theme.obtainStyledAttributes(intArrayOf(android.R.attr.actionBarSize)) + styledAttributes.getDimension(0, 0f).toDp().also { styledAttributes.recycle() } + } + + TopAppBar( + modifier = Modifier.height(actionBarHeight), // set the height here + title = { Row( - modifier = Modifier - .fillMaxWidth() - .clickable { - (context as? Activity)?.start() - }, - verticalAlignment = Alignment.CenterVertically, - horizontalArrangement = Arrangement.Start + modifier = Modifier.fillMaxSize(), + verticalAlignment = Alignment.CenterVertically ) { - Image( - painter = painterResource(R.drawable.ic_feedback_orange_24dp), - contentDescription = "Feedback", - modifier = Modifier - .padding(16.dp) - ) - Text( - text = stringResource(R.string.send_report), - color = if (isDarkTheme) Color.LightGray else Color.DarkGray, - fontSize = 18.sp + modifier = Modifier.padding( + start = dimensionResource(R.dimen.activity_horizontal_margin) + ), + text = stringResource(id = R.string.menu_help), + color = Color.White, + fontWeight = FontWeight.SemiBold ) } - - LazyColumn( - modifier = Modifier - .fillMaxWidth() - ) { - itemsIndexed(data, key = { _, item -> item.title }) { index, item -> - HorizontalDivider( - color = dividerColor - ) - HelpScreenItem(data = item) - } - item { - HorizontalDivider( - color = dividerColor + }, + navigationIcon = { + Row(modifier = Modifier.fillMaxHeight(), verticalAlignment = Alignment.CenterVertically) { + IconButton(onClick = navController::popBackStack) { + Icon( + imageVector = Icons.AutoMirrored.Filled.ArrowBack, + contentDescription = "Back_Navigation", + tint = Color.White ) } } + + }, + colors = TopAppBarDefaults.topAppBarColors( + containerColor = Color.Black + ) + ) +} + +@Composable +fun HelpContent( + data: List, + dividerColor: Color, + innerPadding: androidx.compose.foundation.layout.PaddingValues +) { + Column( + modifier = Modifier + .padding(innerPadding) + ) { + SendReportRow() + HelpItemList(data, dividerColor) + } +} + +@Composable +fun SendReportRow() { + val context = LocalContext.current + val isDarkTheme = isSystemInDarkTheme() + + Row( + modifier = Modifier + .fillMaxWidth() + .clickable { + (context as? Activity)?.start() + }, + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.Start + ) { + Image( + painter = painterResource(R.drawable.ic_feedback_orange_24dp), + contentDescription = stringResource(R.string.send_report), + modifier = Modifier + .padding(dimensionResource(R.dimen.activity_horizontal_margin)) + ) + + Text( + text = stringResource(R.string.send_report), + color = if (isDarkTheme) Color.LightGray else Color.DarkGray, + fontSize = SendDiagnosticReportFontSize + ) + } +} + +@Composable +fun HelpItemList(data: List, dividerColor: Color) { + LazyColumn( + modifier = Modifier + .fillMaxWidth() + ) { + itemsIndexed(data, key = { _, item -> item.title }) { _, item -> + HorizontalDivider( + color = dividerColor + ) + HelpScreenItem(data = item) + } + item { + HorizontalDivider( + color = dividerColor + ) } } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreenItem.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreenItem.kt index e9a90cdb1..a15ab7847 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreenItem.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreenItem.kt @@ -46,10 +46,23 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.graphicsLayer +import androidx.compose.ui.res.dimensionResource +import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp +import org.kiwix.kiwixmobile.core.R + +// Define constants for spacing, font sizes, etc. + +private val HelpItemTitleFontSize = 22.sp +private val HelpItemDescriptionFontSize = 17.sp +private val IconSize = 36.dp +private const val HelpItemAnimationDuration = 300 +private const val HelpItemArrowRotationOpen = 180f +private const val HelpItemArrowRotationClosed = 0f @Composable fun HelpScreenItem( @@ -58,68 +71,82 @@ fun HelpScreenItem( initiallyOpened: Boolean = false ) { var isOpen by remember { mutableStateOf(initiallyOpened) } - val isDarkTheme = isSystemInDarkTheme() - val itemColor = if (isDarkTheme) Color.White else Color.Black - val arrowRotation by animateFloatAsState( - targetValue = if (isOpen) 180f else 0f, - animationSpec = tween(300), - label = "arrowRotation" - ) + val itemColor = if (isSystemInDarkTheme()) Color.White else Color.Black - val interactionSource = remember(::MutableInteractionSource) + val topPadding: Dp = dimensionResource(id = R.dimen.dimen_medium_padding) + val horizontalPadding: Dp = dimensionResource(id = R.dimen.activity_horizontal_margin) Column( modifier = modifier - .fillMaxWidth() - .padding(top = 12.dp), + .fillMaxWidth(), verticalArrangement = Arrangement.Center, horizontalAlignment = Alignment.CenterHorizontally ) { - Row( - horizontalArrangement = Arrangement.SpaceBetween, - verticalAlignment = Alignment.CenterVertically, - modifier = Modifier - .fillMaxWidth() - .clickable(interactionSource = interactionSource, indication = null, onClick = { - isOpen = !isOpen - }) - .padding(horizontal = 16.dp) - ) { - Text( - text = data.title, - fontSize = 18.sp, - color = itemColor, - fontWeight = FontWeight.SemiBold - ) - Icon( - imageVector = Icons.Default.KeyboardArrowDown, - contentDescription = "Open or Close DropDown", - modifier = Modifier - .graphicsLayer { - rotationZ = arrowRotation - } - .size(46.dp), - tint = itemColor - ) - } - - Spacer(modifier = Modifier.height(12.dp)) - + HelpItemHeader(data.title, isOpen, itemColor, horizontalPadding) { isOpen = !isOpen } AnimatedVisibility(visible = isOpen) { - Box( - contentAlignment = Alignment.Center, - modifier = Modifier - .fillMaxWidth() - .padding(start = 16.dp, end = 16.dp) - ) { - Text( - text = data.description, - fontSize = 16.sp, - textAlign = TextAlign.Left, - color = itemColor, - modifier = Modifier.padding(bottom = 8.dp) - ) - } + Spacer(modifier = Modifier.height(topPadding)) + HelpItemDescription(data.description, itemColor, horizontalPadding) } } } + +@Composable +fun HelpItemHeader( + title: String, + isOpen: Boolean, + itemColor: Color, + horizontalPadding: Dp, + onToggle: () -> Unit +) { + val arrowRotation by animateFloatAsState( + targetValue = if (isOpen) HelpItemArrowRotationOpen else HelpItemArrowRotationClosed, + animationSpec = tween(HelpItemAnimationDuration), + label = "arrowRotation" + ) + val interactionSource = remember(::MutableInteractionSource) + + Row( + horizontalArrangement = Arrangement.SpaceBetween, + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier + .fillMaxWidth() + .clickable(interactionSource = interactionSource, indication = null, onClick = onToggle) + .padding(horizontal = horizontalPadding, vertical = horizontalPadding) + ) { + Text( + text = title, + fontSize = HelpItemTitleFontSize, + color = itemColor, + fontWeight = FontWeight.Normal + ) + Icon( + imageVector = Icons.Default.KeyboardArrowDown, + contentDescription = stringResource(R.string.expand), + modifier = Modifier + .graphicsLayer { + rotationZ = arrowRotation + } + .size(IconSize), + tint = itemColor + ) + } +} + +@Composable +fun HelpItemDescription(description: String, itemColor: Color, horizontalPadding: Dp) { + Box( + contentAlignment = Alignment.Center, + modifier = Modifier + .fillMaxWidth() + .padding(start = horizontalPadding, end = horizontalPadding) + ) { + Text( + text = description, + fontSize = HelpItemDescriptionFontSize, + textAlign = TextAlign.Left, + color = itemColor, + modifier = Modifier.padding(bottom = horizontalPadding), + fontWeight = FontWeight.Normal + ) + } +} diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreenItemDataClass.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreenItemDataClass.kt index 6e6fb74af..083d4238e 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreenItemDataClass.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/help/HelpScreenItemDataClass.kt @@ -18,5 +18,5 @@ package org.kiwix.kiwixmobile.core.help -//same as HelpItem data class in HelpAdapter.kt +// same as HelpItem data class in earlier in XML data class HelpScreenItemDataClass(val title: String, val description: String) diff --git a/custom/src/main/java/org/kiwix/kiwixmobile/custom/help/CustomHelpFragment.kt b/custom/src/main/java/org/kiwix/kiwixmobile/custom/help/CustomHelpFragment.kt index 32c898824..b24cad1a4 100644 --- a/custom/src/main/java/org/kiwix/kiwixmobile/custom/help/CustomHelpFragment.kt +++ b/custom/src/main/java/org/kiwix/kiwixmobile/custom/help/CustomHelpFragment.kt @@ -20,4 +20,32 @@ package org.kiwix.kiwixmobile.custom.help import org.kiwix.kiwixmobile.core.help.HelpFragment -class CustomHelpFragment : HelpFragment() +class CustomHelpFragment : HelpFragment() { + override val navHostFragmentId: Int + get() = org.kiwix.kiwixmobile.custom.R.id.custom_nav_controller + + override fun rawTitleDescriptionMap() = + if (sharedPreferenceUtil.isPlayStoreBuildWithAndroid11OrAbove()) { + listOf( + org.kiwix.kiwixmobile.core.R.string.help_2 to + org.kiwix.kiwixmobile.core.R.array.description_help_2, + org.kiwix.kiwixmobile.core.R.string.help_5 to + org.kiwix.kiwixmobile.core.R.array.description_help_5, + org.kiwix.kiwixmobile.core.R.string.how_to_update_content to + org.kiwix.kiwixmobile.core.R.array.update_content_description, + org.kiwix.kiwixmobile.core.R.string.why_copy_move_files_to_app_directory to + getString( + org.kiwix.kiwixmobile.core.R.string.copy_move_files_to_app_directory_description + ) + ) + } else { + listOf( + org.kiwix.kiwixmobile.core.R.string.help_2 to + org.kiwix.kiwixmobile.core.R.array.description_help_2, + org.kiwix.kiwixmobile.core.R.string.help_5 to + org.kiwix.kiwixmobile.core.R.array.description_help_5, + org.kiwix.kiwixmobile.core.R.string.how_to_update_content to + org.kiwix.kiwixmobile.core.R.array.update_content_description + ) + } +} diff --git a/lintConfig.xml b/lintConfig.xml index 44af8acfc..5ef86f071 100644 --- a/lintConfig.xml +++ b/lintConfig.xml @@ -1,5 +1,6 @@ +