[JS Addons] list addons from addons directory #10985

krmanik · 2022-04-20T07:39:11Z

Pull Request template

Purpose / Description

This is UI for addons listing in recycler view from addons directory. The list in view will contains details, remove and toggle switch button.
Also addons menu added to side bar but it will be hidden till all the PR merged.
The implementation will be used in next PR when addons listed from network for download and install.

Fixes

Fixes #7959

Approach

dist package.json made empty when package.json loaded from local directory.
Popup window class created to show info related the current clicked addons
Added a class to list addons from directory, the validated AddonModel added to adapter
Each view in list have three buttons
details button - show popup with addons info like name, author, version etc.
remove button - remove the addons from directory
toggle switch - used to enable/disable addons, for enabled addons, the content will be added to reviewer or note editor

How Has This Been Tested?

Tested on emulator and devices. (Test will be added in next PR)

Listing of empty folder
Listing when one addon added manually (currently manually, in next PR download and install will be implemented)
Info popup window test
Toggle switch test
Delete test

demo.mp4

Learning (optional, can help others)

Checklist

Please, go through these checks before submitting the PR.

You have not changed whitespace unnecessarily (it makes diffs hard to read)
You have a descriptive commit message with a short title (first line, max 50 chars).
Your code follows the style of the project (e.g. never omit braces in if statements)
You have commented your code, particularly in hard-to-understand areas
You have performed a self-review of your own code
UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
UI Changes: You have tested your change using the Google Accessibility Scanner

package.json in tgz file does not contains dist but it is available when the file loaded from network the dist contains .tgz url which will be used to download the file

…ecycler view fix

recycler view used to list addons with delete, details and toggle switch button. delete button - remove the addons from directory details button - show popup with addons info like name, author, version etc. toggle switch - used to enable/disable addons, for enabled addons, the content will be added to reviewer or note editor

and show menu in debug mode

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonsBrowser.kt

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonsBrowserAdapter.kt

…ch toggle status

AnkiDroid/src/main/res/layout/addon_details.xml

AnkiDroid/src/main/res/menu/navigation_drawer.xml

mikehardy

A bunch of unrelated comments :-), I don't see anything really wrong here - a minor structural question, some Kotlin null-typing stuff and some grammar bikeshedding ... Curious for thoughts on all of them

AnkiDroid/src/main/res/values/02-strings.xml

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonsBrowser.kt

mikehardy · 2022-04-24T15:37:10Z

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonsBrowser.kt

+        supportActionBar!!.setHomeButtonEnabled(true)
+        supportActionBar!!.setDisplayHomeAsUpEnabled(true)
+        supportActionBar!!.title = getString(R.string.javascript_addons)


I hate to see new Kotlin code with !! in it, is there any way to avoid this? "There is no way to avoid this" may be a valid answer, I'm just asking/hoping

I have tried but there is no option here. Either it should be wrapped in if/else. I have taken this from another class. So, I need to check more implementation for this.

For another issue if you agree @mikehardy

Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.java

Lines 652 to 660 in 5da9561

protected void enableToolbar() {

Toolbar toolbar = findViewById(R.id.toolbar);

if (toolbar != null) {

setSupportActionBar(toolbar);

} else {

// likely missing "<include layout="@layout/toolbar" />"

Timber.w("Could not find toolbar");

}

}

If we change enableToolbar to return supportActionBar!! (or throw otherwise - the invalid case should be an error), we could:

enableToolbar().apply { setHomeButtonEnabled(true) setDisplayHomeAsUpEnabled(true) title = getString(R.string.javascript_addons) }

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonsBrowser.kt

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonsBrowserAdapter.kt

mikehardy · 2022-04-24T15:48:19Z

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonsBrowserAdapter.kt

+    private fun deleteSelectedAddonPackageDir(addonModel: AddonModel) {
+        // remove the js addon folder
+        val currentAnkiDroidDirectory = CollectionHelper.getCurrentAnkiDroidDirectory(context)
+        val addonsHomeDir = File(currentAnkiDroidDirectory, "addons")
+        val dir = File(addonsHomeDir, addonModel.name)
+
+        val deleted = BackupManager.removeDir(dir)
+


I don't want to over-engineer it, but I feel like having the filesystem code mixed in with the UI code will make it harder to test etc - you have AddonModel, Is there an AddonStorage perhaps, where the "get me the add on directory / get me the list of addons in directory / delete the addon" code could go? Then it would be really easy to have a test just for that stuff

I have created AddonStorage class. It is more cleaner and better implementation. Thanks

mikehardy · 2022-04-24T17:52:37Z

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonsBrowserAdapter.kt

+     *
+     * @param addonModel
+     */
+    private fun deleteSelectedAddonPackageDir(addonModel: AddonModel) {


I think delete can go in the new extracted AddOnStorage object now

mikehardy

I think it's really close now - can put more into the storage class and use it by reference, and should probably do a second refactor as a follow-on to have code from previous PRs use the storage class as well and centralize all the directory / file handling (from the download/install/unpack PRs)

krmanik · 2022-04-24T18:12:07Z

I think this is good idea to handle the addons related file/folder related from a centralized class. I will do refactor in next PR. (Added to TODO)

mikehardy · 2022-05-10T15:40:38Z

I'm going to move this to draft just as a strategy to manage the PR review queue, and because I know you are focused on exams right now (as you should be!). Cheers

krmanik · 2022-05-11T04:54:22Z

I will update the PR later.

github-actions · 2022-07-10T05:37:19Z

Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

github-actions · 2022-12-12T11:36:46Z

Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

mikehardy · 2022-12-13T16:11:13Z

Not stale - dormant because of too much work on my end and prioritizing scoped storage but this will all happen. JS addons are the future and this is the infrastructure...

github-actions · 2023-02-16T13:40:53Z

Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

github-actions · 2023-03-05T19:36:26Z

Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

mikehardy · 2023-03-05T20:39:48Z

The JS AddOn series needs to stay open. It is perfect for 2.17 once storage is finally done

krmanik · 2023-03-06T21:49:49Z

The JS AddOn series needs to stay open. It is perfect for 2.17 once storage is finally done

I will start implementing this after scope storage.

lukstbit · 2024-10-04T08:51:19Z

@krmanik If you want I can look into fixing and updating the code in this PR to get it merged. Maybe the other related PR as well.

mikehardy · 2024-10-04T15:58:21Z

Pains me greatly that these aren't merged yet and I feel a lot of responsibility there - not sure how @krmanik feels about it but I would owe you or anyone a debt of gratitude if the work here was finally shepherded in. It's good stuff IMHO and would be a big step forward I think

david-allison · 2024-10-05T06:13:28Z

@BrayanDSO I believe you had the last conversation regarding addons in the forums. Pinging for your understanding & thoughts

BrayanDSO · 2024-10-06T10:27:04Z

Reviewer add-ons won't happen in the desktop version if they aren't 99,99% secure, and Damien apparently doesn't want it even as a per-notetype opt-in setting with a big warning or just stopped replying because he's busy with something else.

To avoid duplicating the bulletproofing job in the mobile versions, the reviewer should ideally be moved to Svelte first.

I do think that there should be a JS API and addons infrastructure as soon as possible so we can take advantage of the work of other developers. If we wait until all of those desktop issues to be solved (probably at least 3 years from now if I'm optimistic with the current development pace), we will lose a lot of work that could be done in the addons ecosystem in that meantime.

There are a lot of developers, including myself, that may have some time and motivation right now, but not in the future. I could have implemented the whole infrastructure when I created the forums post, but I lost two weeks just in discussion, and stopped getting replied. Now I don't have time for it, nor I want to do it anymore in the future.

So I strongly suggest to continue the JS addons work here in AnkiDroid. To avoid issues in the future, like when Anki implemented a different TTS syntax, I suggest consulting Damien and making him compromise with the API, with whatever is saved in the collection, and with the way that addons are distributed.
That way, things won't break if he wants to distribute things in GitHub with zip files instead of npm with tar.gz, or save the addons list ordered by creation date instead of ID (just an invented example, idk what is planned).

krmanik added 4 commits April 20, 2022 14:48

make dist empty when package loaded from local directory

ca948bd

package.json in tgz file does not contains dist but it is available when the file loaded from network the dist contains .tgz url which will be used to download the file

popup window which shows addons info when details button clicked in r…

7e6ed50

…ecycler view fix

hide addons menu in sidebar in release mode

8cf509f

and show menu in debug mode

krmanik force-pushed the list-addons-from-dir branch from 00ebeac to 8cf509f Compare April 20, 2022 07:55

david-allison requested changes Apr 20, 2022

View reviewed changes

krmanik added 4 commits April 20, 2022 19:57

visibleIf to handle visibility of View

d0e8a6c

load even if addonModel is null and show test when fail

81c235e

make var lateinit and removed !!

77522ce

naming function delete selected addon package dir and simplified swit…

4f0b897

…ch toggle status

krmanik added the Needs Review label Apr 23, 2022

mikehardy reviewed Apr 24, 2022

View reviewed changes

AnkiDroid/src/main/res/layout/addon_details.xml Show resolved Hide resolved

mikehardy reviewed Apr 24, 2022

View reviewed changes

AnkiDroid/src/main/res/menu/navigation_drawer.xml Show resolved Hide resolved

mikehardy requested changes Apr 24, 2022

View reviewed changes

krmanik added 4 commits April 25, 2022 00:54

class for getting addons related directory

30953da

use AddonStorage to get dirs and rename functions and var

ea38699

quote addon name

7de6c8f

escape quote required

98672f6

mikehardy reviewed Apr 24, 2022

View reviewed changes

mikehardy added the Needs Author Reply Waiting for a reply from the original author label Apr 24, 2022

move delete addon dir function to AddonStorage

e92d5a8

krmanik added the Blocked by dependency Currently blocked by some other dependent / related change label Apr 25, 2022

mikehardy marked this pull request as draft May 10, 2022 15:40

github-actions bot added the Stale label Jul 10, 2022

krmanik removed the Stale label Jul 10, 2022

github-actions bot added the Stale label Dec 12, 2022

github-actions bot removed the Stale label Dec 18, 2022

github-actions bot added the Stale label Feb 16, 2023

david-allison removed the Stale label Feb 16, 2023

github-actions bot added the Stale label Mar 5, 2023

mikehardy added Keep Open avoids the stale bot and removed Stale labels Mar 5, 2023

github-actions bot added the Has Conflicts label Dec 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JS Addons] list addons from addons directory #10985

[JS Addons] list addons from addons directory #10985

krmanik commented Apr 20, 2022 •

edited

Loading

mikehardy left a comment

mikehardy Apr 24, 2022

krmanik Apr 24, 2022

david-allison Apr 24, 2022 •

edited

Loading

mikehardy Apr 24, 2022

krmanik Apr 24, 2022

mikehardy Apr 24, 2022

mikehardy left a comment

krmanik commented Apr 24, 2022

mikehardy commented May 10, 2022

krmanik commented May 11, 2022

github-actions bot commented Jul 10, 2022

github-actions bot commented Dec 12, 2022

mikehardy commented Dec 13, 2022

github-actions bot commented Feb 16, 2023

github-actions bot commented Mar 5, 2023

mikehardy commented Mar 5, 2023

krmanik commented Mar 6, 2023

lukstbit commented Oct 4, 2024

mikehardy commented Oct 4, 2024

david-allison commented Oct 5, 2024

BrayanDSO commented Oct 6, 2024

	protected void enableToolbar() {
	Toolbar toolbar = findViewById(R.id.toolbar);
	if (toolbar != null) {
	setSupportActionBar(toolbar);
	} else {
	// likely missing "<include layout="@layout/toolbar" />"
	Timber.w("Could not find toolbar");
	}
	}

[JS Addons] list addons from addons directory #10985

Are you sure you want to change the base?

[JS Addons] list addons from addons directory #10985

Conversation

krmanik commented Apr 20, 2022 • edited Loading

Pull Request template

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Learning (optional, can help others)

Checklist

mikehardy left a comment

Choose a reason for hiding this comment

mikehardy Apr 24, 2022

Choose a reason for hiding this comment

krmanik Apr 24, 2022

Choose a reason for hiding this comment

david-allison Apr 24, 2022 • edited Loading

Choose a reason for hiding this comment

mikehardy Apr 24, 2022

Choose a reason for hiding this comment

krmanik Apr 24, 2022

Choose a reason for hiding this comment

mikehardy Apr 24, 2022

Choose a reason for hiding this comment

mikehardy left a comment

Choose a reason for hiding this comment

krmanik commented Apr 24, 2022

mikehardy commented May 10, 2022

krmanik commented May 11, 2022

github-actions bot commented Jul 10, 2022

github-actions bot commented Dec 12, 2022

mikehardy commented Dec 13, 2022

github-actions bot commented Feb 16, 2023

github-actions bot commented Mar 5, 2023

mikehardy commented Mar 5, 2023

krmanik commented Mar 6, 2023

lukstbit commented Oct 4, 2024

mikehardy commented Oct 4, 2024

david-allison commented Oct 5, 2024

BrayanDSO commented Oct 6, 2024

krmanik commented Apr 20, 2022 •

edited

Loading

david-allison Apr 24, 2022 •

edited

Loading