Added import/export to html functionality #54

GalievBulat · 2024-03-20T11:01:31Z

I've made the requested functionality, while preserving the original code style. I've made the import/export setting close to the import/export setting of json files functionality. Feel free to make changes to the code/design.

app/src/main/java/com/amrdeveloper/linkhub/ui/importexporthtml/ImportExportFragmentHtml.kt

app/src/main/java/com/amrdeveloper/linkhub/ui/importexporthtml/ImportExportHtmlViewModel.kt

AmrDeveloper · 2024-03-20T20:12:55Z

Hello @GalievBulat,

The code looks great for me but we need to make the design more simpler so we can add more extensions on the same view

Maybe Allow the intent to select json or html and depending on the file we select parser, or make the Import button open a dialog or spinner with options

GalievBulat · 2024-03-24T05:32:11Z

@AmrDeveloper yes, I see your point: I think it would be a better decision to implement the functionality on the single fragment, and then change the type of import/export, according to the user's preferences

GalievBulat · 2024-03-24T07:25:21Z

@AmrDeveloper in new commit I've moved import/export functionality into a single abstract Parser class, so check it out, please

app/src/main/res/layout/fragment_setting.xml

app/src/main/res/navigation/nav_graph.xml

moved import/export data handling logic to view model

GalievBulat · 2024-03-25T13:28:12Z

@AmrDeveloper thank you, I think the requested changes are important to do: so I've made a file type picking dialog on an ImportExportFragment. So when user clicks import/export he first needs to pick a file type first

AmrDeveloper · 2024-03-26T20:20:31Z

app/src/main/java/com/amrdeveloper/linkhub/data/ImportExportFileType.kt

+package com.amrdeveloper.linkhub.data
+
+enum class ImportExportFileType(val mimeType: String, val extension: String, val fileTypeName: String) {
+


Please remove empty line

AmrDeveloper · 2024-03-26T20:21:09Z

app/src/main/java/com/amrdeveloper/linkhub/data/source/local/FolderLocalDataSource.kt

@@ -34,6 +34,13 @@ class FolderLocalDataSource internal constructor(
            Result.failure(e)
        }
    }
+    override suspend fun getFolderByName(name : String): Result<Folder> = withContext(ioDispatcher) {


Add new line before method declaration

AmrDeveloper · 2024-03-26T20:21:53Z

app/src/main/java/com/amrdeveloper/linkhub/ui/importexport/ImportExportFragment.kt

@@ -42,11 +43,19 @@ class ImportExportFragment : Fragment() {

    private fun setupListeners() {
        binding.importAction.setOnClickListener {
-            importDataFile()
+            context?.let {  ImportExportFileTypePickerDialog.launch(it) { fileType ->


Please format the code

context?.let { ImportExportFileTypePickerDialog.launch(it) { fileType -> importExportViewModel.setFileType(fileType) importDataFile() } }

AmrDeveloper · 2024-03-26T20:24:41Z

app/src/main/java/com/amrdeveloper/linkhub/ui/importexport/ImportExportViewModel.kt

@@ -29,73 +29,75 @@ class ImportExportViewModel @Inject constructor (
 ) : ViewModel() {

    private val _stateMessages = MutableLiveData<Int>()
+    private var importExportFileParser: ImportExportFileParser = JsonImportExportFileParser()


We don't need to create instance of JsonImportExportFileParser then depending on type we create another parser instance so lets pass FileType to import and export functions and use the correct parser for this type

It will make code simpler and save memory

AmrDeveloper · 2024-03-26T20:25:42Z

app/src/main/java/com/amrdeveloper/linkhub/ui/importexport/ImportExportViewModel.kt

+    val mimeType: String
+        get() =
+            importExportFileParser.getFileType().mimeType
+    val extension: String
+        get() =
+            importExportFileParser.getFileType().extension


Remove those variables and pass the selected FileType to the place we need it

AmrDeveloper · 2024-03-26T20:26:05Z

app/src/main/java/com/amrdeveloper/linkhub/ui/importexport/ImportExportViewModel.kt

-                // Import theme flag if it available
-                val lastThemeOption = uiPreferences.getThemeType()
-                uiPreferences.setThemeType(dataPackage.theme ?: lastThemeOption)
+    fun setFileType(fileType: ImportExportFileType){


Remove this function and pass the FileType directly to import / export functions

AmrDeveloper · 2024-03-26T20:29:01Z

@AmrDeveloper thank you, I think the requested changes are important to do: so I've made a file type picking dialog on an ImportExportFragment. So when user clicks import/export he first needs to pick a file type first

Thank you for update, there are some design points and feel free to discuss your idea and suggestion

My point is that import and export functions on view model should take the type and do their work, later they will pass this type to factory for example

val parser = FileParserFactory.getInstance(fileType)
parser.import() or parser.export()

The view model will not know which parser it used and don't save the file type

GalievBulat · 2024-03-27T09:34:32Z

@AmrDeveloper thanks for the suggested changes: I've change the code according to them, so please review the commit

AmrDeveloper · 2024-03-27T10:29:44Z

app/src/main/java/com/amrdeveloper/linkhub/ui/importexport/ImportExportViewModel.kt

@@ -28,79 +26,77 @@ class ImportExportViewModel @Inject constructor (
    private val uiPreferences: UiPreferences,
 ) : ViewModel() {

+    private var importExportFileType: ImportExportFileType? = null


Now we don't need this variable

AmrDeveloper · 2024-03-27T10:30:46Z

app/src/main/java/com/amrdeveloper/linkhub/util/ImportExportFileTypePickerDialog.kt

+object ImportExportFileTypePickerDialog {
+
+    fun launch(context: Context, onFileTypeSelected: (ImportExportFileType)->Unit) {
+        val fileTypes = ImportExportFileType.entries.map { it.fileTypeName }
+        val builder = AlertDialog.Builder(context)
+        builder.setTitle(context.getString(R.string.import_export_choose_file_type))
+        builder.setItems(fileTypes.toTypedArray()) { dialog, which ->
+            val selectedFileType = ImportExportFileType.entries[which]
+            onFileTypeSelected(selectedFileType)
+            dialog.dismiss()
+        }
+        val dialog = builder.create()
+        dialog.show()
+    }
+}


We can move this to a function in ImportExport settings because it's only needed their launchImportExportFileTypePicker

AmrDeveloper · 2024-03-27T10:32:32Z

app/src/main/java/com/amrdeveloper/linkhub/data/parser/ImportExportFileParser.kt

+    object ImportExportFileParserFactory {
+        fun getInstance(fileType: ImportExportFileType): ImportExportFileParser {
+            return when (fileType) {
+                ImportExportFileType.JSON -> JsonImportExportFileParser()
+                ImportExportFileType.HTML -> HtmlImportExportFileParser()
+            }
+        }
+    }


You can create the factory pattern but as a single function in the view model for now

fun getDataParser(fileType: ImportExportFileType): ImportExportFileParser { return when (fileType) { ImportExportFileType.JSON -> JsonImportExportFileParser() ImportExportFileType.HTML -> HtmlImportExportFileParser() } }

AmrDeveloper · 2024-03-27T10:33:46Z

app/src/main/java/com/amrdeveloper/linkhub/ui/importexport/ImportExportFragment.kt

@@ -42,11 +45,21 @@ class ImportExportFragment : Fragment() {

    private fun setupListeners() {
        binding.importAction.setOnClickListener {
-            importDataFile()
+            context?.let {


After converting this picker to function you can use requireContext() it will return non nullable context to you to use directly

AmrDeveloper · 2024-03-27T10:34:55Z

app/src/main/java/com/amrdeveloper/linkhub/ui/importexport/ImportExportFragment.kt

@@ -23,6 +25,7 @@ import dagger.hilt.android.AndroidEntryPoint
 @AndroidEntryPoint
 class ImportExportFragment : Fragment() {

+    private var importExportFileType: ImportExportFileType? = null


Can be remove this variable and pass the state between functions?

I think there the state of importExportFileType cannot be omitted because registerForActivityResult should be declared before they fragment is created, so I think we cannot pass the state between functions

Okey lets keep it for now

AmrDeveloper · 2024-03-27T10:36:09Z

Thank you @GalievBulat for changes, i have a little changes in places and file structure and everything else is good for me,

GalievBulat · 2024-03-27T12:31:08Z

I've made the requested changes, so please check once again

AmrDeveloper · 2024-03-27T20:23:38Z

Thank you for great work @GalievBulat

Added import/export to html functionality

1094b8e

AmrDeveloper requested changes Mar 20, 2024

View reviewed changes

app/src/main/java/com/amrdeveloper/linkhub/ui/importexporthtml/ImportExportFragmentHtml.kt Outdated Show resolved Hide resolved

app/src/main/java/com/amrdeveloper/linkhub/ui/importexporthtml/ImportExportHtmlViewModel.kt Outdated Show resolved Hide resolved

moved import/export functionality into a single fragment

98cb753

GalievBulat requested a review from AmrDeveloper March 25, 2024 07:45

AmrDeveloper requested changes Mar 25, 2024

View reviewed changes

app/src/main/res/layout/fragment_setting.xml Outdated Show resolved Hide resolved

app/src/main/res/layout/fragment_setting.xml Outdated Show resolved Hide resolved

app/src/main/res/navigation/nav_graph.xml Show resolved Hide resolved

Galiev Bulat added 2 commits March 25, 2024 16:21

added import/export file picking dialog

e73d21a

moved import/export data handling logic to view model

added import/export file picking dialog on an export button click

98a0684

GalievBulat requested a review from AmrDeveloper March 25, 2024 13:28

AmrDeveloper requested changes Mar 26, 2024

View reviewed changes

refactored the code design

be3d0fb

GalievBulat requested a review from AmrDeveloper March 27, 2024 09:34

AmrDeveloper requested changes Mar 27, 2024

View reviewed changes

refactored the code design

f882898

GalievBulat requested a review from AmrDeveloper March 27, 2024 12:31

AmrDeveloper approved these changes Mar 27, 2024

View reviewed changes

AmrDeveloper merged commit ae2c071 into AmrDeveloper:master Mar 27, 2024
2 of 3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added import/export to html functionality #54

Added import/export to html functionality #54

GalievBulat commented Mar 20, 2024

AmrDeveloper commented Mar 20, 2024 •

edited

Loading

GalievBulat commented Mar 24, 2024

GalievBulat commented Mar 24, 2024

GalievBulat commented Mar 25, 2024

AmrDeveloper Mar 26, 2024

AmrDeveloper Mar 26, 2024

AmrDeveloper Mar 26, 2024

AmrDeveloper Mar 26, 2024

AmrDeveloper Mar 26, 2024

AmrDeveloper Mar 26, 2024

AmrDeveloper commented Mar 26, 2024

GalievBulat commented Mar 27, 2024

AmrDeveloper Mar 27, 2024

AmrDeveloper Mar 27, 2024

AmrDeveloper Mar 27, 2024

AmrDeveloper Mar 27, 2024

AmrDeveloper Mar 27, 2024

GalievBulat Mar 27, 2024 •

edited

Loading

AmrDeveloper Mar 27, 2024

AmrDeveloper commented Mar 27, 2024

GalievBulat commented Mar 27, 2024

AmrDeveloper commented Mar 27, 2024

		package com.amrdeveloper.linkhub.data

		enum class ImportExportFileType(val mimeType: String, val extension: String, val fileTypeName: String) {

Added import/export to html functionality #54

Added import/export to html functionality #54

Conversation

GalievBulat commented Mar 20, 2024

AmrDeveloper commented Mar 20, 2024 • edited Loading

GalievBulat commented Mar 24, 2024

GalievBulat commented Mar 24, 2024

GalievBulat commented Mar 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmrDeveloper commented Mar 26, 2024

GalievBulat commented Mar 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GalievBulat Mar 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmrDeveloper commented Mar 27, 2024

GalievBulat commented Mar 27, 2024

AmrDeveloper commented Mar 27, 2024

AmrDeveloper commented Mar 20, 2024 •

edited

Loading

GalievBulat Mar 27, 2024 •

edited

Loading