[Feature] Enhanced image viewing#15
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughImplemented an image viewer feature with Viewer.js integration, refactored all templates to use layout inheritance, added fullscreen editor mode with auto-closing HTML tags, enhanced image upload error handling, and improved user feedback with flash messages. Also introduced dynamic timestamp field detection. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser as Browser/<br/>JS
participant App as Flask App
participant Storage as Remote Storage
User->>Browser: Click image in post
activate Browser
Browser->>Browser: Viewer.js initializes<br/>modal overlay
Browser->>Browser: Apply .viewer-backdrop<br/>and .viewer-canvas styles
deactivate Browser
Note over Browser: Modal appears with<br/>image full-screen
User->>Browser: Scroll wheel (ctrl+key)
activate Browser
Browser->>Browser: Zoom image via<br/>Viewer.js zoom API
deactivate Browser
User->>Browser: Click outside / Press Esc
activate Browser
Browser->>Browser: Close modal,<br/>remove overlay
deactivate Browser
sequenceDiagram
actor User as User
participant Editor as Fullscreen Editor
participant Form as Browser Form
participant App as Flask App
User->>Editor: Toggle fullscreen<br/>(click button or URL param)
activate Editor
Editor->>Editor: Toggle .fullscreen class<br/>on UI elements
Editor->>Editor: Update URL to ?fullscreen=true
deactivate Editor
User->>Editor: Type HTML-like tag<br/>(e.g., "<div>")
activate Editor
Editor->>Editor: On ">", auto-insert<br/>closing tag "</div>"
Editor->>Editor: Position cursor<br/>between tags
deactivate Editor
User->>Editor: Press Shift+Enter
activate Editor
Editor->>Form: Gather title & content
Editor->>App: POST /new with FormData
deactivate Editor
App-->>Editor: Success response
activate Editor
alt Fullscreen mode
Editor->>Editor: Clear editor
else Normal mode
Editor->>Editor: Navigate to /new
end
deactivate Editor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Rationale: The changes span multiple file types (backend, templates, CSS, JS) with mixed complexity. Template refactoring is repetitive but requires verification of block structure consistency across files. Image viewer integration is straightforward Viewer.js usage. Fullscreen editor and error handling logic are moderately complex. Backend timestamp detection and image upload error handling require careful attention. Test additions are straightforward but comprehensive. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
static/js/script.js (1)
37-37: Fix typo in event listener.Line 37 has a typo: "DOMContentLaded" should be "DOMContentLoaded". This means the
checkSessionfunction is never called on page load.Apply this diff:
-document.addEventListener("DOMContentLaded", checkSession); +document.addEventListener("DOMContentLoaded", checkSession);app.py (1)
343-357: Fix unsafe exception attribute access.Line 346 attempts to access
int(e.status)on a generic exception, but Python's baseExceptionclass doesn't have astatusattribute. This will raise anAttributeErrorif the Supabase exception doesn't have this attribute.Apply this defensive fix:
except Exception as e: print(f"Error uploading image to Superbase: {e}") try: - if int(e.status) == 409: + if hasattr(e, 'status') and int(e.status) == 409: image_url = f"{SUPABASE_URL}/storage/v1/object/public/{BLOG_IMAGES_BUCKET}/{filename}" else: os.makedirs(app.config['UPLOAD_FOLDER'], exist_ok=True) local_path = os.path.join(app.config['UPLOAD_FOLDER'], filename) with open(local_path, 'wb') as out_f: out_f.write(file_bytes) image_url = f"/static/uploads/{filename}" print(f"Saved image locally to {local_path}; using {image_url} as image URL") except Exception as e2: print(f"Failed to save image locally: {e2}") image_url = None
🧹 Nitpick comments (7)
static/js/new.js (1)
49-51: Clarify popup display logic.The popup is shown only when entering fullscreen (line 50), but the timeout to hide it (lines 59-61) runs unconditionally. This means the popup briefly appears and disappears even when exiting fullscreen, which may not be the intended behavior.
Consider moving the timeout inside the conditional:
if (editor.classList.contains("full-screen")){ - popup.classList.add('show'); + popup.classList.add('show'); + setTimeout(() => { + popup.classList.remove('show'); + }, 1500); } if (container.classList.contains("full-screen")) { window.history.pushState({}, '', '?fullscreen=true'); } else { window.history.pushState({}, '', window.location.pathname); } - - setTimeout(() => { - popup.classList.remove('show'); - }, 1500); }static/css/style.css (1)
622-624: Consider scoping cursor: pointer to clickable images only.Setting
cursor: pointeron allimgelements (line 622) may give misleading affordances if some images are not interactive (e.g., logos, icons, decorative images).Consider scoping the cursor to images within galleries or with a specific class:
-img { +.gallery img { cursor: pointer; }This assumes all clickable images are within
.gallerycontainers, as indicated by the viewer initialization logic in static/js/script.js.templates/layout.html (1)
11-11: Consider adding defer or async to script.js.The script at line 11 loads in the
<head>withoutdeferorasync, which may block HTML parsing and delay page rendering.Apply this diff to improve page load performance:
- <script src="{{ url_for('static', filename='js/script.js') }}"></script> + <script src="{{ url_for('static', filename='js/script.js') }}" defer></script>Using
deferensures the script executes after the DOM is parsed, which is appropriate since the script contains DOMContentLoaded listeners.static/js/editor.js (2)
29-31: Clarify or remove unclear Backspace/Delete handler.The handler for Backspace and Delete keys simply returns without performing any action or preventing default behavior. If this is intentional for future functionality, add a comment explaining the purpose. Otherwise, remove it.
63-93: Consider using this file only for edit operations.The Shift+Enter handler submits to
/newendpoint (line 73), but this file is loaded bytemplates/edit.htmlfor editing posts. This creates confusion about the file's purpose. Consider:
- Renaming to clarify scope (e.g.,
editor-shared.js)- Or splitting edit-specific and new-post-specific logic into separate files
templates/post.html (2)
6-6: Consider moving stylesheet link to base layout.Loading the post-specific CSS inside the content block is unconventional. While functional, it's better practice to include stylesheets in the
<head>section. Consider adding astylesblock inlayout.htmlor moving this to the base template's head.
11-16: Move inline styles to CSS file.Line 14 uses an inline style
width: 90%. This should be moved tostatic/css/post.cssfor better maintainability and separation of concerns.In
static/css/post.css:.gallery .image { width: 90%; }Then remove the inline style:
- <img src="{{ post.image }}" alt="Post Image" class="image" - style="width: 90%" /> + <img src="{{ post.image }}" alt="Post Image" class="image" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/test_viewer_image.pngis excluded by!**/*.png
📒 Files selected for processing (14)
app.py(2 hunks)static/css/post.css(1 hunks)static/css/style.css(1 hunks)static/js/editor.js(1 hunks)static/js/new.js(1 hunks)static/js/script.js(1 hunks)templates/edit.html(1 hunks)templates/index.html(1 hunks)templates/layout.html(1 hunks)templates/new.html(1 hunks)templates/post.html(1 hunks)tests/test_blog.py(1 hunks)tests/test_edit_post.py(2 hunks)tests/test_viewer.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/test_blog.py (1)
tests/conftest.py (1)
page(54-59)
tests/test_viewer.py (2)
tests/conftest.py (3)
page(54-59)flask_app_url(16-51)admin_logged_in_page(63-72)static/js/script.js (2)
image(89-89)viewer(90-95)
tests/test_edit_post.py (1)
tests/conftest.py (1)
page(54-59)
🪛 Ruff (0.14.0)
app.py
408-408: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (16)
tests/test_blog.py (1)
101-101: LGTM! Test selector updated to match new HTML structure.The change from
imgto.imageclass selector aligns with the UI refactor where images now use a container with the.imageclass.tests/test_edit_post.py (1)
115-115: LGTM! Test selectors updated consistently.The changes from
imgto.imageclass selector are consistent with the UI refactor and align with the changes in tests/test_blog.py.Also applies to: 131-131
static/css/style.css (1)
610-620: LGTM! Image viewer styling is well-implemented.The viewer backdrop and canvas styling effectively uses CSS variables, includes Safari prefixes, and provides a polished visual experience with blur effects and shadows.
static/js/script.js (2)
72-82: LGTM! Filter toggle function works correctly.The
toggleFilter()function appropriately toggles the panel state and updates button text. The implementation is straightforward and functional.
84-103: LGTM! Image viewer initialization is well-structured.The viewer initialization correctly:
- Iterates over gallery elements
- Configures Viewer with appropriate options
- Prevents default click behavior
- Shows the viewer on image click
templates/new.html (1)
1-23: LGTM! Clean refactor to layout inheritance.The template successfully:
- Extends the base layout
- Moves inline JavaScript to an external file (static/js/new.js)
- Maintains all form fields and functionality
- Loads the script at the end of the content block for proper DOM availability
This improves maintainability and follows best practices for template organization.
templates/layout.html (2)
17-25: LGTM! Toast container implementation is clean.The toast container correctly:
- Uses Flask's
get_flashed_messageswith categories- Imports and renders the toast component
- Provides a fixed position container for notifications
12-13: Verify and regenerate Viewer.js CDN integrity hashes using srihash.org.The CDNJS supplied SRI hashes do not always match the actual CDN files, as documented in GitHub issue #14124. The template currently uses hardcoded SHA-512 hashes for Viewer.js v1.11.7 (CSS and JS). srihash.org is an easy-to-use tool to generate SRI hashes.
Confirm the integrity hashes are correct before deployment, or the Viewer.js assets may fail to load in browsers with SRI verification enabled.
static/css/post.css (1)
1-58: LGTM! Post page styling is well-structured.The CSS file:
- Consistently uses CSS variables from the design system
- Provides responsive adjustments for mobile screens
- Includes appropriate shadows and border radius for visual polish
- Properly styles the
.imageclass referenced by teststemplates/edit.html (1)
1-28: LGTM! Clean template refactoring.The template correctly extends the base layout and maintains all necessary functionality. The separation of JavaScript into an external file (editor.js) improves maintainability.
app.py (2)
55-75: LGTM! Smart dynamic timestamp detection.The
resolve_timestamp_field()function elegantly handles different timestamp field naming conventions across database schemas. The fallback to 'timestamp' ensures robustness.
407-410: LGTM! Good user feedback additions.The flash messages provide clear feedback to users about post deletion success or failure. This aligns well with the toast notification system introduced in the layout template.
tests/test_viewer.py (3)
67-107: Creative zoom verification approach!The bounding box measurement technique for verifying zoom functionality is clever, especially given that the toolbar is disabled. This provides robust verification without relying on UI controls.
1-107: LGTM! Comprehensive viewer test coverage.The test suite effectively covers:
- Viewer activation on homepage
- Viewer activation on post detail page
- Zoom functionality verification
Tests follow Playwright best practices with proper fixtures, expectations, and cleanup.
8-27: Test image file is present in version control.The verification confirms that
tests/test_viewer_image.pngexists at the expected location. The code will function properly without test failures related to missing image files.templates/index.html (1)
1-73: No issues found—JavaScript functions are properly defined.Both
toggleFilter()andlogoutUser()functions referenced in the template's onclick handlers are correctly defined instatic/js/script.js. The JavaScript dependencies are satisfied.
| image_url = f"{SUPABASE_URL}/storage/v1/object/public/{BLOG_IMAGES_BUCKET}/{filename}" | ||
| except Exception as e: | ||
| print(f"Error uploading image to Superbase: {e}") | ||
| print(f"Error uploading image to Superbase: {e}") |
There was a problem hiding this comment.
Fix typo in error message.
Line 344 contains "Superbase" instead of "Supabase".
- print(f"Error uploading image to Superbase: {e}")
+ print(f"Error uploading image to Supabase: {e}") 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| print(f"Error uploading image to Superbase: {e}") | |
| print(f"Error uploading image to Supabase: {e}") |
🤖 Prompt for AI Agents
In app.py around line 344, the error message prints "Superbase" instead of
"Supabase"; update the string to "Error uploading image to Supabase: {e}" (fix
the typo) and scan nearby log messages for any other "Superbase" occurrences and
correct them to "Supabase".
| editor.addEventListener("keydown", function (event) { | ||
| const cursorPos = editor.selectionStart; | ||
| const value = editor.value; | ||
|
|
||
| // Handle auto-closing only when typing '>' | ||
| if (event.key === ">" && cursorPos > 1) { | ||
| const beforeCursor = value.substring(0, cursorPos); | ||
| const match = beforeCursor.match(/<([a-zA-Z0-9]+)$/); | ||
|
|
||
| if (match) { | ||
| event.preventDefault(); | ||
| const tag = match[1]; | ||
| const closingTag = `>${`</${tag}>`}`; | ||
|
|
||
| editor.value = value.substring(0, cursorPos) + closingTag + value.substring(cursorPos); | ||
| editor.selectionStart = editor.selectionEnd = cursorPos + 1; | ||
| } | ||
| } | ||
|
|
||
| if (event.key === "Backspace" || event.key === "Delete") { | ||
| return; | ||
| } | ||
| }); | ||
|
|
||
| function FullScreen() { | ||
| const container = document.querySelector(".form"); | ||
| const button = document.querySelector(".form").querySelector("button"); | ||
| const full_screen_button = document.querySelector(".full-screen-button"); | ||
| const form_text_input = document.querySelector(".form-text-input"); | ||
| const editor = document.getElementById("editor"); | ||
| const form_file_input = document.querySelector(".form-file-input"); | ||
| const popup = document.getElementById('popup'); | ||
|
|
||
| container.classList.toggle("full-screen"); | ||
| button.classList.toggle("full-screen"); | ||
| full_screen_button.classList.toggle("full-screen"); | ||
| form_text_input.classList.toggle("full-screen"); | ||
| form_file_input.classList.toggle("full-screen"); | ||
| editor.classList.toggle("full-screen"); | ||
| if (editor.classList.contains("full-screen")) { | ||
| popup.classList.add('show'); | ||
| } | ||
| if (container.classList.contains("full-screen")) { | ||
| window.history.pushState({}, '', '?fullscreen=true'); | ||
| } else { | ||
| window.history.pushState({}, '', window.location.pathname); | ||
| } | ||
|
|
||
|
|
||
| setTimeout(() => { | ||
| popup.classList.remove('show'); | ||
| }, 1500); | ||
| } | ||
| editor.addEventListener('keydown', function (event) { | ||
| if (event.shiftKey && event.key === 'Enter') { | ||
| event.preventDefault(); | ||
|
|
||
| const content = editor.value; | ||
| const title = document.querySelector('input[name="title"]').value; | ||
| const formData = new FormData(); | ||
| formData.append('title', title); | ||
| formData.append('content', content); | ||
|
|
||
| fetch('/new', { | ||
| method: 'POST', | ||
| body: formData, | ||
| }) | ||
| .then(response => { | ||
| if (response.ok) { | ||
| console.log('Post submitted successfully.'); | ||
| if (editor.classList.contains("full-screen")) { | ||
| editor.value = ''; | ||
| } else { | ||
| window.location.href = '/new'; | ||
| } | ||
| } else { | ||
| alert('Error while submitting the post.'); | ||
| } | ||
| }) | ||
| .catch(error => { | ||
| console.error('Error:', error); | ||
| alert('Something went wrong while submitting the post.'); | ||
| }); | ||
| } | ||
| if (event.key === 'Escape') { | ||
| if (editor.classList.contains("full-screen")) { | ||
| FullScreen(); | ||
| } | ||
| } | ||
| }); No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Consolidate duplicate event listeners.
Two separate keydown event listeners are attached to the editor element (lines 10 and 63). This creates confusion and potential conflicts. Combine them into a single handler.
Merge both handlers:
editor.addEventListener("keydown", function (event) {
const cursorPos = editor.selectionStart;
const value = editor.value;
// Handle auto-closing only when typing '>'
if (event.key === ">" && cursorPos > 1) {
const beforeCursor = value.substring(0, cursorPos);
const match = beforeCursor.match(/<([a-zA-Z0-9]+)$/);
if (match) {
event.preventDefault();
const tag = match[1];
const closingTag = `></${tag}>`;
editor.value = value.substring(0, cursorPos) + closingTag + value.substring(cursorPos);
editor.selectionStart = editor.selectionEnd = cursorPos + 1;
}
}
- if (event.key === "Backspace" || event.key === "Delete") {
- return;
- }
-});
-
-function FullScreen() {
- // ... (keep FullScreen function)
-}
-
-editor.addEventListener('keydown', function (event) {
+ // Handle Shift+Enter for post submission
if (event.shiftKey && event.key === 'Enter') {
event.preventDefault();
const content = editor.value;
const title = document.querySelector('input[name="title"]').value;
const formData = new FormData();
formData.append('title', title);
formData.append('content', content);
fetch('/new', {
method: 'POST',
body: formData,
})
.then(response => {
if (response.ok) {
console.log('Post submitted successfully.');
if (editor.classList.contains("full-screen")) {
editor.value = '';
} else {
window.location.href = '/new';
}
} else {
alert('Error while submitting the post.');
}
})
.catch(error => {
console.error('Error:', error);
alert('Something went wrong while submitting the post.');
});
}
+
+ // Handle Escape to exit fullscreen
if (event.key === 'Escape') {
if (editor.classList.contains("full-screen")) {
FullScreen();
}
}
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In static/js/editor.js around lines 10 to 99 there are two separate
editor.keydown handlers; merge them into one to avoid conflicts: keep the
auto-closing tag logic (trigger only on key === '>' with the match and
preventDefault, insert closing tag and set caret) but do that after early-return
guards for Backspace/Delete; include the Shift+Enter form submit logic
(preventDefault, build FormData, fetch and handle success/error and clearing
editor only when in full-screen) and the Escape key handling (toggle FullScreen
if in full-screen). Ensure the merged handler reuses the existing editor
variable, preserves event.preventDefault calls where used, maintains
caret/selection updates, and keeps ordering such that special keys
(Backspace/Delete) return early, '>' handling runs before form submission on
Shift+Enter, and Escape handling runs last.
| // Handle auto-closing only when typing '>' | ||
| if (event.key === ">" && cursorPos > 1) { | ||
| const beforeCursor = value.substring(0, cursorPos); | ||
| const match = beforeCursor.match(/<([a-zA-Z0-9]+)$/); | ||
|
|
||
| if (match) { | ||
| event.preventDefault(); | ||
| const tag = match[1]; | ||
| const closingTag = `>${`</${tag}>`}`; | ||
|
|
||
| editor.value = value.substring(0, cursorPos) + closingTag + value.substring(cursorPos); | ||
| editor.selectionStart = editor.selectionEnd = cursorPos + 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix syntax error in auto-closing tag logic.
Line 22 contains a malformed template literal. The syntax >${`</${tag}>`}` will result in incorrect string concatenation.
Apply this fix:
- const closingTag = `>${`</${tag}>`}`;
+ const closingTag = `></${tag}>`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Handle auto-closing only when typing '>' | |
| if (event.key === ">" && cursorPos > 1) { | |
| const beforeCursor = value.substring(0, cursorPos); | |
| const match = beforeCursor.match(/<([a-zA-Z0-9]+)$/); | |
| if (match) { | |
| event.preventDefault(); | |
| const tag = match[1]; | |
| const closingTag = `>${`</${tag}>`}`; | |
| editor.value = value.substring(0, cursorPos) + closingTag + value.substring(cursorPos); | |
| editor.selectionStart = editor.selectionEnd = cursorPos + 1; | |
| } | |
| } | |
| // Handle auto-closing only when typing '>' | |
| if (event.key === ">" && cursorPos > 1) { | |
| const beforeCursor = value.substring(0, cursorPos); | |
| const match = beforeCursor.match(/<([a-zA-Z0-9]+)$/); | |
| if (match) { | |
| event.preventDefault(); | |
| const tag = match[1]; | |
| const closingTag = `></${tag}>`; | |
| editor.value = value.substring(0, cursorPos) + closingTag + value.substring(cursorPos); | |
| editor.selectionStart = editor.selectionEnd = cursorPos + 1; | |
| } | |
| } |
🤖 Prompt for AI Agents
static/js/editor.js lines 14-27: The auto-closing tag logic uses a malformed
template literal; replace the broken expression with a proper closing-tag
string. Set closingTag to the closing tag constructed from the captured tag (for
example the string "</" + tag + ">" i.e. </tag>), then insert that string at the
cursor and keep the selection logic as-is.
| if (match) { | ||
| event.preventDefault(); | ||
| const tag = match[1]; | ||
| const closingTag = `>${`</${tag}>`}`; |
There was a problem hiding this comment.
Fix the template literal syntax error.
The string concatenation on line 22 has invalid syntax. The closing tag should be a plain string, not nested template literals.
Apply this diff to fix the syntax:
- const closingTag = `>${`</${tag}>`}`;
+ const closingTag = `></${tag}>`;🤖 Prompt for AI Agents
In static/js/new.js around line 22 the closingTag assignment uses nested
template literals causing a syntax error; replace it with a single string that
represents the closing HTML tag by interpolating the tag variable in one
template literal (i.e. backticks with </ then the tag then >) or by
concatenating the pieces into a normal string ('</' + tag + '>') so the closing
tag is a valid plain string.
| function FullScreen() { | ||
| const container = document.querySelector(".form"); | ||
| const button = document.querySelector(".form").querySelector("button"); | ||
| const full_screen_button = document.querySelector(".full-screen-button"); | ||
| const form_text_input = document.querySelector(".form-text-input"); | ||
| const editor = document.getElementById("editor"); | ||
| const form_file_input = document.querySelector(".form-file-input"); | ||
| const popup = document.getElementById('popup'); | ||
|
|
||
| container.classList.toggle("full-screen"); | ||
| button.classList.toggle("full-screen"); | ||
| full_screen_button.classList.toggle("full-screen"); | ||
| form_text_input.classList.toggle("full-screen"); | ||
| form_file_input.classList.toggle("full-screen"); | ||
| editor.classList.toggle("full-screen"); | ||
| if (editor.classList.contains("full-screen")){ | ||
| popup.classList.add('show'); | ||
| } | ||
| if (container.classList.contains("full-screen")) { | ||
| window.history.pushState({}, '', '?fullscreen=true'); | ||
| } else { | ||
| window.history.pushState({}, '', window.location.pathname); | ||
| } | ||
|
|
||
|
|
||
| setTimeout(() => { | ||
| popup.classList.remove('show'); | ||
| }, 1500); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add null checks for DOM elements.
The FullScreen() function queries multiple DOM elements without null checks. If any element is missing (e.g., due to template changes or loading order issues), the function will throw runtime errors.
Apply this diff to add defensive checks:
function FullScreen() {
const container = document.querySelector(".form");
const button = document.querySelector(".form").querySelector("button");
const full_screen_button = document.querySelector(".full-screen-button");
const form_text_input = document.querySelector(".form-text-input");
const editor = document.getElementById("editor");
const form_file_input = document.querySelector(".form-file-input");
const popup = document.getElementById('popup');
+
+ if (!container || !button || !full_screen_button || !form_text_input || !editor || !form_file_input || !popup) {
+ console.error('FullScreen: Required elements not found');
+ return;
+ }
container.classList.toggle("full-screen");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function FullScreen() { | |
| const container = document.querySelector(".form"); | |
| const button = document.querySelector(".form").querySelector("button"); | |
| const full_screen_button = document.querySelector(".full-screen-button"); | |
| const form_text_input = document.querySelector(".form-text-input"); | |
| const editor = document.getElementById("editor"); | |
| const form_file_input = document.querySelector(".form-file-input"); | |
| const popup = document.getElementById('popup'); | |
| container.classList.toggle("full-screen"); | |
| button.classList.toggle("full-screen"); | |
| full_screen_button.classList.toggle("full-screen"); | |
| form_text_input.classList.toggle("full-screen"); | |
| form_file_input.classList.toggle("full-screen"); | |
| editor.classList.toggle("full-screen"); | |
| if (editor.classList.contains("full-screen")){ | |
| popup.classList.add('show'); | |
| } | |
| if (container.classList.contains("full-screen")) { | |
| window.history.pushState({}, '', '?fullscreen=true'); | |
| } else { | |
| window.history.pushState({}, '', window.location.pathname); | |
| } | |
| setTimeout(() => { | |
| popup.classList.remove('show'); | |
| }, 1500); | |
| } | |
| function FullScreen() { | |
| const container = document.querySelector(".form"); | |
| const button = document.querySelector(".form").querySelector("button"); | |
| const full_screen_button = document.querySelector(".full-screen-button"); | |
| const form_text_input = document.querySelector(".form-text-input"); | |
| const editor = document.getElementById("editor"); | |
| const form_file_input = document.querySelector(".form-file-input"); | |
| const popup = document.getElementById('popup'); | |
| if (!container || !button || !full_screen_button || !form_text_input || !editor || !form_file_input || !popup) { | |
| console.error('FullScreen: Required elements not found'); | |
| return; | |
| } | |
| container.classList.toggle("full-screen"); | |
| button.classList.toggle("full-screen"); | |
| full_screen_button.classList.toggle("full-screen"); | |
| form_text_input.classList.toggle("full-screen"); | |
| form_file_input.classList.toggle("full-screen"); | |
| editor.classList.toggle("full-screen"); | |
| if (editor.classList.contains("full-screen")){ | |
| popup.classList.add('show'); | |
| } | |
| if (container.classList.contains("full-screen")) { | |
| window.history.pushState({}, '', '?fullscreen=true'); | |
| } else { | |
| window.history.pushState({}, '', window.location.pathname); | |
| } | |
| setTimeout(() => { | |
| popup.classList.remove('show'); | |
| }, 1500); | |
| } |
🤖 Prompt for AI Agents
In static/js/new.js around lines 34 to 62, the FullScreen() function calls
querySelector/getElementById and immediately accesses properties on the returned
nodes, which will throw if any element is missing; update the function to guard
every DOM lookup with a null check before toggling classes or calling methods
(e.g., only call classList.toggle/classList.contains and history.pushState when
the element exists), bail out early or skip operations for missing elements, and
ensure the popup timeout is set only if popup was found.
| editor.addEventListener('keydown', function (event) { | ||
| if (event.shiftKey && event.key === 'Enter') { | ||
| event.preventDefault(); | ||
|
|
||
| const content = editor.value; | ||
| const title = document.querySelector('input[name="title"]').value; | ||
| const formData = new FormData(); | ||
| formData.append('title', title); | ||
| formData.append('content', content); | ||
|
|
||
| fetch('/new', { | ||
| method: 'POST', | ||
| body: formData, | ||
| }) | ||
| .then(response => { | ||
| if (response.ok) { | ||
| console.log('Post submitted successfully.'); | ||
| if (editor.classList.contains("full-screen")) { | ||
| editor.value = ''; | ||
| } else { | ||
| window.location.href = '/new'; | ||
| } | ||
| } else { | ||
| alert('Error while submitting the post.'); | ||
| } | ||
| }) | ||
| .catch(error => { | ||
| console.error('Error:', error); | ||
| alert('Something went wrong while submitting the post.'); | ||
| }); |
There was a problem hiding this comment.
Image upload not included in fetch submission.
The Shift+Enter submission (lines 63-92) uses FormData but only appends title and content. The image file input is not included, so users cannot upload images via this shortcut.
Consider whether image upload should be supported via Shift+Enter submission. If yes, apply this diff:
const content = editor.value;
const title = document.querySelector('input[name="title"]').value;
+ const imageInput = document.querySelector('input[name="image"]');
const formData = new FormData();
formData.append('title', title);
formData.append('content', content);
+ if (imageInput.files.length > 0) {
+ formData.append('image', imageInput.files[0]);
+ }If image upload via Shift+Enter is not intended, consider adding user feedback (e.g., a toast notification) to inform users that only text content will be submitted.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| editor.addEventListener('keydown', function (event) { | |
| if (event.shiftKey && event.key === 'Enter') { | |
| event.preventDefault(); | |
| const content = editor.value; | |
| const title = document.querySelector('input[name="title"]').value; | |
| const formData = new FormData(); | |
| formData.append('title', title); | |
| formData.append('content', content); | |
| fetch('/new', { | |
| method: 'POST', | |
| body: formData, | |
| }) | |
| .then(response => { | |
| if (response.ok) { | |
| console.log('Post submitted successfully.'); | |
| if (editor.classList.contains("full-screen")) { | |
| editor.value = ''; | |
| } else { | |
| window.location.href = '/new'; | |
| } | |
| } else { | |
| alert('Error while submitting the post.'); | |
| } | |
| }) | |
| .catch(error => { | |
| console.error('Error:', error); | |
| alert('Something went wrong while submitting the post.'); | |
| }); | |
| editor.addEventListener('keydown', function (event) { | |
| if (event.shiftKey && event.key === 'Enter') { | |
| event.preventDefault(); | |
| const content = editor.value; | |
| const title = document.querySelector('input[name="title"]').value; | |
| const imageInput = document.querySelector('input[name="image"]'); | |
| const formData = new FormData(); | |
| formData.append('title', title); | |
| formData.append('content', content); | |
| if (imageInput.files.length > 0) { | |
| formData.append('image', imageInput.files[0]); | |
| } | |
| fetch('/new', { | |
| method: 'POST', | |
| body: formData, | |
| }) | |
| .then(response => { | |
| if (response.ok) { | |
| console.log('Post submitted successfully.'); | |
| if (editor.classList.contains("full-screen")) { | |
| editor.value = ''; | |
| } else { | |
| window.location.href = '/new'; | |
| } | |
| } else { | |
| alert('Error while submitting the post.'); | |
| } | |
| }) | |
| .catch(error => { | |
| console.error('Error:', error); | |
| alert('Something went wrong while submitting the post.'); | |
| }); |
🤖 Prompt for AI Agents
In static/js/new.js around lines 63-92 the FormData only includes title and
content so image inputs are not uploaded when users press Shift+Enter; fix by
selecting the image file input (e.g., document.querySelector for the file input
by name or id), check if it has files and if so append the file(s) to formData
(append a single file as 'image' or loop append multiple with the same key),
then proceed with fetch; alternatively, if image upload via Shift+Enter is
intentionally unsupported, add a small user-facing notification before
submission to inform users that images will not be uploaded by this shortcut.
1 participant
📌 Summary
In this pull request, I have added the Lightroom effect for better image viewing. I have also moved some of the
CSSandJavaScriptto their own files for clarity, and I have written some tests for them. And also fixed a little bug in the image uploading behaviour.🔧 Type of Change
🧩 Related Issues
Closes #11
🧠 Changes Introduced
viewer.js.📸 video
image-viewing.mp4
✅ Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Style
Tests