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.

Sign up for GitHub

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

Mobile: Resolves #8639: implement callback url #9803

Merged
merged 10 commits into from
Jun 15, 2024
+75 −0
Merged

Mobile: Resolves #8639: implement callback url #9803

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6b23817
Mobile: resolves #8639: implement callback url on android
tiberiusteng Jan 30, 2024
4e1729d
cleanup
tiberiusteng Jan 30, 2024
37364c5
go back and close side menu before navigating to callback url
tiberiusteng Feb 5, 2024
1fa5809
add Edit/Delete notebook menu items in right-top three dots menu
tiberiusteng Mar 3, 2024
f81f73a
Merge remote-tracking branch 'origin/dev' into android-callback-url
tiberiusteng Mar 4, 2024
a0f555f
Merge remote-tracking branch 'origin/dev' into android-callback-url
tiberiusteng Mar 9, 2024
f58bb1e
Merge remote-tracking branch 'origin/dev' into android-callback-url
tiberiusteng Mar 14, 2024
4f8f1a8
Merge remote-tracking branch 'origin/dev' into android-callback-url
tiberiusteng Mar 31, 2024
5eb0019
Merge remote-tracking branch 'origin/dev' into android-callback-url
tiberiusteng Jun 11, 2024
fed843c
remove menus from Notes screen
tiberiusteng Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/app-mobile/android/app/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,15 @@
<data android:mimeType="*/*" />
</intent-filter>
<!-- /SHARE EXTENSION -->

<!-- EXTERNAL LINK -->
<intent-filter>
<action android:name="android.intent.action.VIEW" />
<category android:name="android.intent.category.DEFAULT" />
<category android:name="android.intent.category.BROWSABLE" />
<data android:scheme="joplin" />
</intent-filter>
<!-- /EXTERNAL LINK -->
</activity>

</application>
Expand Down
12 changes: 12 additions & 0 deletions packages/app-mobile/components/screens/Note.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import { join } from 'path';
import { Dispatch } from 'redux';
import { RefObject } from 'react';
import { SelectionRange } from '../NoteEditor/types';
import { getNoteCallbackUrl } from '@joplin/lib/callbackUrlUtils';
const urlUtils = require('@joplin/lib/urlUtils');

const emptyArray: any[] = [];
Expand Down Expand Up @@ -1098,6 +1099,11 @@ class NoteScreenComponent extends BaseScreenComponent<Props, State> implements B
Clipboard.setString(Note.markdownTag(note));
}

private copyExternalLink_onPress() {
const note = this.state.note;
Clipboard.setString(getNoteCallbackUrl(note.id));
}

public sideMenuOptions() {
const note = this.state.note;
if (!note) return [];
Expand Down Expand Up @@ -1305,6 +1311,12 @@ class NoteScreenComponent extends BaseScreenComponent<Props, State> implements B
this.copyMarkdownLink_onPress();
},
});
output.push({
title: _('Copy external link'),
onPress: () => {
this.copyExternalLink_onPress();
},
});
}
output.push({
title: _('Properties'),
Expand Down
29 changes: 27 additions & 2 deletions packages/app-mobile/components/screens/Notes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import Tag from '@joplin/lib/models/Tag';
import Note from '@joplin/lib/models/Note';
import Setting from '@joplin/lib/models/Setting';
const { themeStyle } = require('../global-style.js');
import { ScreenHeader } from '../ScreenHeader';
import { ScreenHeader, MenuOptionType } from '../ScreenHeader';
import { _ } from '@joplin/lib/locale';
import ActionButton from '../ActionButton';
const { dialogs } = require('../../utils/dialogs.js');
Expand All @@ -18,6 +18,8 @@ const { BackButtonService } = require('../../services/back-button.js');
import { AppState } from '../../utils/types';
import { NoteEntity } from '@joplin/lib/services/database/types';
const { ALL_NOTES_FILTER_ID } = require('@joplin/lib/reserved-ids.js');
const Clipboard = require('@react-native-community/clipboard').default;
import { getFolderCallbackUrl, getTagCallbackUrl } from '@joplin/lib/callbackUrlUtils';

class NotesScreenComponent extends BaseScreenComponent<any> {

Expand Down Expand Up @@ -197,6 +199,29 @@ class NotesScreenComponent extends BaseScreenComponent<any> {
return this.folderPickerOptions_;
}

public menuOptions() {
const output: MenuOptionType[] = [];

if (this.props.notesParentType === 'Tag') {
output.push({
title: _('Copy external link'),
onPress: () => {
Clipboard.setString(getTagCallbackUrl(this.props.selectedTagId));
},
});
}
if (this.props.notesParentType === 'Folder') {
output.push({
title: _('Copy external link'),
onPress: () => {
Clipboard.setString(getFolderCallbackUrl(this.props.selectedFolderId));
},
});
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with the existing UI, this menu option should appear when long pressing a folder in the sidebar.

Same for the tags, although currently we don't have a menu there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
Do you mean this dialog? Add a new "Copy external link" button?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using the mobile app, I never thought to long press on the item in the sidebar, or to long press the tag in the list when I want to look for the "Copy external link" function.

I used "Copy markdown link" function before, it's in the right-top menu button, so I looked there and add the function there. Folder/tag node list didn't have that menu button before but I think it's natural to look for the function at the same place. Mental model ls like "copy the link to current screen".

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, but the problem is that we'd now have similar functionalities in two different places. Either we move all the long press features to the kebab menu, or we move that copy external link item to the long press one.

Copy link
Contributor Author

@tiberiusteng tiberiusteng Feb 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this menu option should appear when long pressing a folder in the sidebar.

I need clarification on this, since in the mobile app what I found while long pressing a folder in the sidebar is the alert dialog I took screenshot above.

Tags doesn't show in the sidebar, but in a tag list on the right side (I'm not sure what that part called). Long-pressing a tag currently shows nothing.

Long-pressing a note will change the title bar to 'Move to notebook ...' dropdown, select all button, delete button, copy button; there's no space for additional buttons or existing menu to add new action to.

I can't think a way to put 'Copy external link' into these places that feels 'consistent'. What should it 'consistent' with?

Right-click menus appear in Desktop application but mobile app doesn't have it. Desktop app already have 'Copy external link' there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I've tried to implement that but my knowledge/ability on React-Native isn't good enough to finish it (yet) ... so it probably need to be implemented later by somebody else.

Personally I still think putting the "Copy external link" option on the top-right dotted menu is the most intuitive place, because they appear at the same place for note, (note list of) tag, (note list of) notebook, and serve the idea of "Copy link to current screen", instead of scattering across:

  • For note, top-right dotted menu
  • For notebook, long-press notebook name in sidebar menu (it looks like menu in iOS but a strange dialog in Android, should be fixed anyway)
  • For tag, long-press tag name in tag list (which don't do anything right now)

Anyway the core part is handling external links on mobile, the menus are nice to have but perhaps I've done a little too much. If it's not desired I can add a new commit to remove them, or just don't merge Note.tsx and Notes.tsx.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I think it's reasonable too to have this context menu as you suggest. But in that case would you be able to remove the long-press menu and move the options over to that kebab menu?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I'll try it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tiberiusteng. It looks like you made further changes and added the kebab menu, but the long press menu is still there? If could remove it now that you moved the functionalities that would great.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that removing existing interface will cause confusion to users used that function before (but I don't think of a good way to direct users from original position to the new position, maybe just let them co-exist a few versions). But if you are OK to remove it now it's great too. I will wait for further discussion or final confirmation.


return output;
}

public render() {
const parent = this.parentItem();
const theme = themeStyle(this.props.themeId);
Expand Down Expand Up @@ -285,7 +310,7 @@ class NotesScreenComponent extends BaseScreenComponent<any> {
accessibilityElementsHidden={accessibilityHidden}
importantForAccessibility={accessibilityHidden ? 'no-hide-descendants' : undefined}
>
<ScreenHeader title={iconString + title} showBackButton={false} parentComponent={thisComp} sortButton_press={this.sortButton_press} folderPickerOptions={this.folderPickerOptions()} showSearchButton={true} showSideMenuButton={true} />
<ScreenHeader title={iconString + title} showBackButton={false} parentComponent={thisComp} sortButton_press={this.sortButton_press} folderPickerOptions={this.folderPickerOptions()} showSearchButton={true} showSideMenuButton={true} menuOptions={this.menuOptions()} />
<NoteList />
{actionButtonComp}
<DialogBox
Expand Down
54 changes: 54 additions & 0 deletions packages/app-mobile/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ const SyncTargetDropbox = require('@joplin/lib/SyncTargetDropbox.js');
const SyncTargetAmazonS3 = require('@joplin/lib/SyncTargetAmazonS3.js');
import BiometricPopup from './components/biometrics/BiometricPopup';
import initLib from '@joplin/lib/initLib';
import { isCallbackUrl, parseCallbackUrl, CallbackUrlCommand } from '@joplin/lib/callbackUrlUtils';

SyncTargetRegistry.addClass(SyncTargetNone);
SyncTargetRegistry.addClass(SyncTargetOneDrive);
Expand Down Expand Up @@ -764,6 +765,7 @@ class AppComponent extends React.Component {
private appStateChangeListener_: NativeEventSubscription|null = null;
private themeChangeListener_: NativeEventSubscription|null = null;
private dropdownAlert_ = (_data: any) => new Promise<any>(res => res);
private callbackUrl: string|null = null;

public constructor() {
super();
Expand Down Expand Up @@ -793,6 +795,12 @@ class AppComponent extends React.Component {
if (event.url === ShareExtension.shareURL && this.props.biometricsDone) {
logger.info('Sharing: handleOpenURL_: Processing share data');
void this.handleShareData();
} else if (isCallbackUrl(event.url)) {
logger.info('received callback url: ', event.url);
this.callbackUrl = event.url;
if (this.props.biometricsDone) {
void this.handleCallbackUrl();
}
}
};

Expand Down Expand Up @@ -952,6 +960,7 @@ class AppComponent extends React.Component {
if (this.props.biometricsDone !== prevProps.biometricsDone && this.props.biometricsDone) {
logger.info('Sharing: componentDidUpdate: biometricsDone');
void this.handleShareData();
void this.handleCallbackUrl();
}
}

Expand Down Expand Up @@ -992,6 +1001,51 @@ class AppComponent extends React.Component {
}
}

private async handleCallbackUrl() {
const url = this.callbackUrl;
this.callbackUrl = null;
if (url === null) {
return;
}

const { command, params } = parseCallbackUrl(url);

// adopted from app-mobile/utils/shareHandler.ts
// We go back one screen in case there's already a note open -
// if we don't do this, the dispatch below will do nothing
// (because routeName wouldn't change)
this.props.dispatch({ type: 'NAV_BACK' });
this.props.dispatch({ type: 'SIDE_MENU_CLOSE' });

switch (command) {

case CallbackUrlCommand.OpenNote:
this.props.dispatch({
type: 'NAV_GO',
routeName: 'Note',
noteId: params.id,
});
break;

case CallbackUrlCommand.OpenTag:
this.props.dispatch({
type: 'NAV_GO',
routeName: 'Notes',
tagId: params.id,
});
break;

case CallbackUrlCommand.OpenFolder:
this.props.dispatch({
type: 'NAV_GO',
routeName: 'Notes',
folderId: params.id,
});
break;

}
}

private async handleScreenWidthChange_() {
this.setState({ sideMenuWidth: this.getSideMenuWidth() });
}
Expand Down
Loading