diff --git a/OfflineU-project-summary.md b/OfflineU-project-summary.md index d6f7f7f..ad73c4c 100644 --- a/OfflineU-project-summary.md +++ b/OfflineU-project-summary.md @@ -248,6 +248,29 @@ before. session), including Settings → Test Connection hitting the real HTTP path. +## Outline integration follow-up fixes (this session) + +- **Space bar bug**: the lesson page's global keyboard-shortcut handler + (space/arrows for play-pause/seek/volume) fired regardless of what had + focus, so typing a space in the notes textarea toggled the video instead. + Fixed with an `isTypingTarget()` guard that skips the shortcut handler + entirely when a textarea/input/select/contenteditable has focus. +- **Topic model redesign**: topics used to each spawn their own top-level + Outline *collection*. Changed so a topic is instead a *document* inside + one fixed, user-configured collection (Settings → Outline Integration → + Default collection, e.g. "Training Notes") - a lesson's note becomes a + child document nested under its topic document, matching how the user + actually organizes their real Outline instance. `list_outline_topics()` + fetches the collection's documents and filters to `parentDocumentId is + None` locally rather than trusting `documents.list`'s `collectionId`/ + `parentDocumentId` request filters - both are marked deprecated in + Outline's own API spec with unclear recursive-vs-top-level semantics, so + filtering the response ourselves is the version that can't be wrong. + Verified end-to-end against a local mock Outline server: resolving a new + topic creates a top-level doc, pushing a note creates a child doc under + it, and the child never leaks back into the topic chooser as if it were + a topic itself. + ## Known limitations still open - App is unauthenticated by design (matches upstream) — settings and hidden-path curation apply app-wide, not per-browser/per-user. diff --git a/offlineu_core.py b/offlineu_core.py index 5dfc12d..ebe7391 100644 --- a/offlineu_core.py +++ b/offlineu_core.py @@ -911,7 +911,7 @@ OUTLINE_CONFIG_FILE = os.path.join(DATA_DIR, 'outline_config.json') def load_outline_config() -> Dict[str, str]: - """Load the Outline base URL + API token. Never exposed via GET beyond 'configured'.""" + """Load the Outline base URL + API token + default collection. Never exposed via GET beyond 'configured'.""" try: if os.path.exists(OUTLINE_CONFIG_FILE): with open(OUTLINE_CONFIG_FILE, 'r') as f: @@ -919,20 +919,33 @@ def load_outline_config() -> Dict[str, str]: if isinstance(data, dict): return { 'base_url': data.get('base_url', ''), - 'api_token': data.get('api_token', '') + 'api_token': data.get('api_token', ''), + 'default_collection_id': data.get('default_collection_id', ''), + 'default_collection_name': data.get('default_collection_name', '') } except (json.JSONDecodeError, OSError) as e: print(f"Could not load Outline config: {e}") - return {'base_url': '', 'api_token': ''} + return {'base_url': '', 'api_token': '', 'default_collection_id': '', 'default_collection_name': ''} -def save_outline_config(base_url: str, api_token: str) -> None: - """Save the Outline base URL + API token. A blank api_token keeps the - previously stored one, so the URL can be updated without re-pasting it.""" +def save_outline_config(base_url: str, api_token: str, + default_collection_id: Optional[str] = None, + default_collection_name: Optional[str] = None) -> None: + """ + Save the Outline base URL / API token / default collection. A blank + api_token keeps the previously stored one, so the URL can be updated + without re-pasting it; default_collection_id/name are only touched when + explicitly passed (None means 'leave as-is'), so saving the URL/token + doesn't clear an already-chosen collection. + """ current = load_outline_config() current['base_url'] = base_url.rstrip('/') if api_token: current['api_token'] = api_token + if default_collection_id is not None: + current['default_collection_id'] = default_collection_id + if default_collection_name is not None: + current['default_collection_name'] = default_collection_name os.makedirs(DATA_DIR, exist_ok=True) with open(OUTLINE_CONFIG_FILE, 'w') as f: json.dump(current, f, indent=2) @@ -965,39 +978,72 @@ def _outline_request(path: str, payload: Dict[str, Any]) -> Dict[str, Any]: raise RuntimeError(f'Could not reach Outline: {e.reason}') +def list_outline_topics() -> List[Dict[str, str]]: + """ + Top-level documents in the configured default collection - each one is + a "topic" a lesson note can be filed under. Fetches every document in + the collection and filters to parentDocumentId is None locally, rather + than relying on documents.list's collectionId/parentDocumentId filter + params directly - both are marked deprecated in Outline's own API spec + and their exact recursive-vs-top-level behavior isn't documented, so + filtering the response ourselves is the version that can't be wrong. + """ + config = load_outline_config() + collection_id = config['default_collection_id'] + if not collection_id: + raise RuntimeError('No default Outline collection set - pick one in Settings first') + + result = _outline_request('documents.list', {'collectionId': collection_id, 'limit': 100}) + return [ + {'id': d['id'], 'name': d['title']} + for d in result.get('data', []) + if not d.get('parentDocumentId') + ] + + def resolve_outline_topic(name: str) -> Dict[str, str]: """ - Find an existing Outline collection with this exact name, or create one. - Matching by name first (rather than always creating) means naming a - "new" topic that happens to match one you already made doesn't spawn a - duplicate collection. + Find an existing topic document (a top-level document in the configured + default collection) with this exact title, or create one. Matching by + name first (rather than always creating) means naming a "new" topic + that happens to match one you already made doesn't spawn a duplicate. """ - existing = _outline_request('collections.list', {'limit': 100}) - for c in existing.get('data', []): - if c['name'] == name: - return {'id': c['id'], 'name': c['name']} - created = _outline_request('collections.create', {'name': name, 'permission': 'read_write'}) + for topic in list_outline_topics(): + if topic['name'] == name: + return topic + config = load_outline_config() + created = _outline_request('documents.create', { + 'title': name, + 'text': '', + 'collectionId': config['default_collection_id'], + 'publish': True + }) return {'id': created['data']['id'], 'name': name} def push_note_to_outline(course: Course, lesson_path: str, lesson_title: str, topic_id: str, new_topic_name: str) -> Dict[str, Any]: """ - Push a lesson's saved note to Outline as a document in the chosen topic - (collection). Creates the document on the first push and updates the - same one (by the id stashed in the progress file) on every push after - that. + Push a lesson's saved note to Outline as a child document nested under + the chosen topic document (which itself lives in the configured default + collection - see list_outline_topics). Creates the note document on the + first push and updates the same one (by the id stashed in the progress + file) on every push after that. new_topic_name is a fallback for a topic that somehow never got resolved client-side (see resolve_outline_topic) - the normal path resolves a - brand-new topic name to a real collection id as soon as it's typed + brand-new topic name to a real document id as soon as it's typed (POST /api/outline/resolve-topic), specifically so this fire-and-forget, can't-read-the-response push never has to decide "is this actually a new topic" on its own. A page that fires pagehide more than once for the same view (bfcache restore, for instance) would otherwise re-send - the same "new topic" intent every time and create a duplicate - collection per navigation. + the same "new topic" intent every time and create a duplicate topic + document per navigation. """ + config = load_outline_config() + if not config['default_collection_id']: + return {'success': False, 'error': 'No default Outline collection set - pick one in Settings first'} + progress = ProgressTracker.load_progress(course) entry = progress.get(lesson_path, {}) note = entry.get('note', '') @@ -1018,7 +1064,8 @@ def push_note_to_outline(course: Course, lesson_path: str, lesson_title: str, created = _outline_request('documents.create', { 'title': title, 'text': note, - 'collectionId': topic_id, + 'collectionId': config['default_collection_id'], + 'parentDocumentId': topic_id, 'publish': True }) document_id = created['data']['id'] @@ -1996,20 +2043,29 @@ def update_lesson_note_topic_api(): @app.route('/api/outline/config', methods=['GET']) def get_outline_config_api(): - """Whether Outline is configured, and the (non-secret) base URL - never the token.""" + """Whether Outline is configured, the (non-secret) base URL, and the default collection - never the token.""" config = load_outline_config() - return jsonify({'configured': bool(config['base_url'] and config['api_token']), 'base_url': config['base_url']}) + return jsonify({ + 'configured': bool(config['base_url'] and config['api_token']), + 'base_url': config['base_url'], + 'default_collection_id': config['default_collection_id'], + 'default_collection_name': config['default_collection_name'] + }) @app.route('/api/outline/config', methods=['POST']) def set_outline_config_api(): - """Save the Outline base URL / API token.""" + """Save the Outline base URL / API token / default collection.""" data = request.json or {} base_url = (data.get('base_url') or '').strip() api_token = (data.get('api_token') or '').strip() if not base_url: return jsonify({'error': 'Outline URL is required'}), 400 - save_outline_config(base_url, api_token) + save_outline_config( + base_url, api_token, + default_collection_id=data.get('default_collection_id'), + default_collection_name=data.get('default_collection_name') + ) return jsonify({'success': True}) @@ -2025,7 +2081,7 @@ def test_outline_connection_api(): @app.route('/api/outline/collections') def list_outline_collections_api(): - """The topic chooser's option list - live from Outline, not locally cached.""" + """Every Outline collection - used by the Settings page's default-collection picker.""" try: result = _outline_request('collections.list', {'limit': 100}) except RuntimeError as e: @@ -2034,10 +2090,20 @@ def list_outline_collections_api(): return jsonify({'collections': collections}) +@app.route('/api/outline/topics') +def list_outline_topics_api(): + """The lesson page's topic chooser option list - top-level documents in the configured default collection.""" + try: + topics = list_outline_topics() + except RuntimeError as e: + return jsonify({'topics': [], 'error': str(e)}) + return jsonify({'topics': topics}) + + @app.route('/api/outline/resolve-topic', methods=['POST']) def resolve_outline_topic_api(): """ - Resolve a freshly-typed topic name to a real Outline collection id right + Resolve a freshly-typed topic name to a real Outline document id right away (find-or-create), rather than deferring to the fire-and-forget pagehide push - see push_note_to_outline's docstring for why that matters (a page that fires pagehide more than once, e.g. via bfcache, diff --git a/templates/lesson_view.html b/templates/lesson_view.html index e91936a..4c8cc08 100644 --- a/templates/lesson_view.html +++ b/templates/lesson_view.html @@ -606,11 +606,11 @@ const topicStatus = document.getElementById('outline-topic-status'); if (topicSelect) { - fetch('/api/outline/collections') + fetch('/api/outline/topics') .then(r => r.json()) .then(data => { const newOption = topicSelect.querySelector('option[value="__new__"]'); - (data.collections || []).forEach(function(c) { + (data.topics || []).forEach(function(c) { const opt = document.createElement('option'); opt.value = c.id; opt.textContent = c.name; @@ -624,7 +624,7 @@ newTopicInput.value = INITIAL_TOPIC_NAME; } if (data.error && topicStatus) { - topicStatus.textContent = 'Outline not reachable - topic list may be incomplete'; + topicStatus.textContent = data.error; } }) .catch(() => {}); @@ -805,8 +805,17 @@ }, 3000); } - // Keyboard shortcuts + // Keyboard shortcuts - skip while typing in the note (or any + // other text field), so space/arrow keys type normally instead + // of hijacking video playback. + function isTypingTarget(el) { + if (!el) return false; + const tag = el.tagName; + return tag === 'TEXTAREA' || tag === 'INPUT' || tag === 'SELECT' || el.isContentEditable; + } + document.addEventListener('keydown', function(e) { + if (isTypingTarget(e.target)) return; if (activeMedia && !e.ctrlKey && !e.altKey && !e.metaKey) { switch(e.key) { case ' ': diff --git a/templates/settings.html b/templates/settings.html index ce042a4..76e694b 100644 --- a/templates/settings.html +++ b/templates/settings.html @@ -524,8 +524,9 @@
- Pick a topic on a lesson's note (on the lesson page) to push it to that topic's collection in - Outline when you leave the page. Notes without a topic stay local only. + Pick a topic on a lesson's note (on the lesson page) to push it there when you leave the page - + each topic becomes a document inside the collection below, and the note is nested under it. + Notes without a topic stay local only.