Auto-expanding textarea for frontpage text in event settings #1340
Conversation
Reviewer's GuideIntroduces an auto-expanding internationalized textarea widget and applies it to the event frontpage text setting, including inline JavaScript to dynamically adjust the textarea height for static and dynamically added i18n fields. Sequence diagram for auto-expanding frontpage text textareasequenceDiagram
actor "Organizer" as Organizer
participant "Browser" as Browser
participant "DjangoView" as DjangoView
participant "I18nAutoExpandingTextarea" as Widget
participant "JS_AutoExpand" as JS
"Organizer" ->> "Browser": "Open event settings page"
"Browser" ->> "DjangoView": "GET /event/settings"
"DjangoView" ->> "Widget": "Render field 'frontpage_text' with widget 'I18nAutoExpandingTextarea'"
"Widget" -->> "DjangoView": "HTML <textarea data-auto-expand='true'> + inline <script>"
"DjangoView" -->> "Browser": "Settings page HTML with auto-expanding textarea"
"Browser" ->> "JS": "Trigger 'DOMContentLoaded' event"
"JS" ->> "Browser": "Select 'textarea[data-auto-expand="true"]' and initialize autoExpandTextarea"
"JS" ->> "Browser": "Attach 'input' and 'paste' listeners to textarea"
"Organizer" ->> "Browser": "Type or paste content into frontpage text textarea"
"Browser" ->> "JS": "Fire 'input' or 'paste' event"
"JS" ->> "Browser": "adjustHeight(): set height between 80px and 300px, toggle vertical overflow"
"Browser" ->> "JS": "MutationObserver detects dynamically added i18n textareas (language tabs)"
"JS" ->> "Browser": "Initialize autoExpandTextarea on new i18n textareas with data-auto-expand='true'"
Class diagram for the new I18nAutoExpandingTextarea widgetclassDiagram
class I18nTextarea {
}
class I18nAutoExpandingTextarea {
+ __init__(attrs=None, **kwargs)
+ render(name, value, attrs=None, renderer=None)
- auto-expanding behavior configured via HTML attributes and inline JavaScript
}
I18nTextarea <|-- I18nAutoExpandingTextarea
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The auto-expanding behavior is injected via an inline <script> on every widget render; consider moving this JS into a shared static file or a global initialization hook to avoid repeated scripts and improve maintainability.
- The widget hardcodes presentation details (min/max height and transitions) directly in the style attribute; consider moving these into CSS classes so they can be more easily themed or adjusted without changing Python code.
- In event/settings.html, {% load static %} is now present twice; you can remove the duplicated tag to keep the template tidy.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The auto-expanding behavior is injected via an inline <script> on every widget render; consider moving this JS into a shared static file or a global initialization hook to avoid repeated scripts and improve maintainability.
- The widget hardcodes presentation details (min/max height and transitions) directly in the style attribute; consider moving these into CSS classes so they can be more easily themed or adjusted without changing Python code.
- In event/settings.html, {% load static %} is now present twice; you can remove the duplicated tag to keep the template tidy.
## Individual Comments
### Comment 1
<location> `app/eventyay/base/forms/__init__.py:188-197` </location>
<code_context>
+ def render(self, name, value, attrs=None, renderer=None):
</code_context>
<issue_to_address>
**issue (performance):** Embedding a full <script> block in each widget render may cause duplication and performance overhead.
Since `render` returns `html + js_code`, every widget instance adds another identical `<script>` tag to the DOM. With multiple fields this leads to repeated DOMContentLoaded handlers / observers and inflated HTML. Prefer having the widget only set `data-auto-expand` (and classes) and loading the auto-expand logic once via shared JS or a base template. If you must inline here, add a simple guard (e.g., a global flag on `window`) so the logic runs only once even if the script tag is duplicated.
</issue_to_address>
### Comment 2
<location> `app/eventyay/base/forms/__init__.py:185` </location>
<code_context>
def __init__(self, attrs=None, **kwargs):
default_attrs = {
'class': 'form-control auto-expanding-textarea',
'data-auto-expand': 'true',
'style': 'min-height: 80px; max-height: 300px; overflow-y: hidden; resize: vertical; transition: height 0.2s ease-in-out;'
}
if attrs:
if 'class' in attrs:
default_attrs['class'] = default_attrs['class'] + ' ' + attrs['class']
default_attrs.update(attrs)
super().__init__(attrs=default_attrs, **kwargs)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge dictionary updates via the union operator ([`dict-assign-update-to-union`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/dict-assign-update-to-union/))
```suggestion
default_attrs |= attrs
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR adds an auto-expanding textarea widget for internationalized text fields in event settings, specifically applied to the frontpage text field to improve the editing experience for long descriptions.
Key changes:
- Introduces
I18nAutoExpandingTextareawidget with self-contained JavaScript for dynamic height adjustment - Applies the new widget to the
frontpage_textfield in event settings configuration - Adds template tag loading in the settings template
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| app/eventyay/base/forms/init.py | Adds new I18nAutoExpandingTextarea widget class with inline JavaScript for auto-expanding functionality |
| app/eventyay/base/configurations/default_setting.py | Updates frontpage_text field to use the new I18nAutoExpandingTextarea widget and imports the widget |
| app/eventyay/eventyay_common/templates/eventyay_common/event/settings.html | Adds {% load static %} tag (though it appears to be duplicated) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
app/eventyay/eventyay_common/templates/eventyay_common/event/settings.html
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </fieldset> | ||
| <fieldset> | ||
| <legend>{% trans "Localization" %}</legend> | ||
| {% bootstrap_field sform.locales layout="control" %} |
There was a problem hiding this comment.
The content_locales field has been removed from the Localization section. This appears to be unintentional as it's not mentioned in the PR description and is unrelated to the auto-expanding textarea feature being added.
The removed line was:
{% if sform.content_locales %}{% bootstrap_field sform.content_locales layout="control" %}{% endif %}This should be restored between sform.locales and sform.locale fields to maintain the original functionality.
| {% bootstrap_field sform.locales layout="control" %} | |
| {% bootstrap_field sform.locales layout="control" %} | |
| {% if sform.content_locales %}{% bootstrap_field sform.content_locales layout="control" %}{% endif %} |
| default_attrs = { | ||
| 'class': 'form-control auto-expanding-textarea', | ||
| 'data-auto-expand': 'true', | ||
| 'style': 'min-height: 80px; max-height: 300px; overflow-y: auto; resize: vertical; transition: height 0.2s ease-in-out;' |
There was a problem hiding this comment.
The default style includes overflow-y: auto which conflicts with the dynamic overflow management in the JavaScript. The JavaScript sets overflowY to either 'auto' or 'hidden' based on content height (line 20 of auto-expanding-textarea.js), but the initial overflow-y: auto means scrollbars will be visible before the JavaScript initializes or when content is below the max height.
Consider changing overflow-y: auto to overflow-y: hidden in the default style:
'style': 'min-height: 80px; max-height: 300px; overflow-y: hidden; resize: vertical; transition: height 0.2s ease-in-out;'This will prevent unnecessary scrollbars from appearing initially and let the JavaScript control the overflow behavior.
| 'style': 'min-height: 80px; max-height: 300px; overflow-y: auto; resize: vertical; transition: height 0.2s ease-in-out;' | |
| 'style': 'min-height: 80px; max-height: 300px; overflow-y: hidden; resize: vertical; transition: height 0.2s ease-in-out;' |
|
|
||
|
|
There was a problem hiding this comment.
Trailing whitespace detected on this line. According to PEP 8 style guidelines, lines should not have trailing whitespace. Remove the trailing spaces from this line.
|
|
||
|
|
There was a problem hiding this comment.
Trailing whitespace detected on this line. According to PEP 8 style guidelines, lines should not have trailing whitespace. Remove the trailing spaces from this line.
| class I18nURLFormField(i18nfield.forms.I18nFormField): | ||
| """ | ||
| Custom form field to handle internationalized URL inputs. It extends the I18nFormField | ||
| and ensures that all provided URLs are valid. | ||
| Methods: | ||
| clean(value: LazyI18nString) -> LazyI18nString: | ||
| Validates the URL(s) in the provided internationalized input. | ||
| """ | ||
|
|
||
| def clean(self, value) -> LazyI18nString: | ||
| """ | ||
| Cleans and validates the internationalized URL input. | ||
| Args: | ||
| value (LazyI18nString): The input value to clean and validate. | ||
| Returns: | ||
| LazyI18nString: The cleaned and validated input value. | ||
| Raises: | ||
| ValidationError: If any of the URLs are invalid. | ||
| """ | ||
| value = super().clean(value) | ||
| if not value: | ||
| return value | ||
|
|
||
| url_validator = URLValidator() | ||
|
|
||
| if isinstance(value.data, dict): | ||
| for val in value.data.values(): | ||
| if val: | ||
| url_validator(val) | ||
| else: | ||
| url_validator(value.data) | ||
|
|
||
| return value | ||
| return value |
There was a problem hiding this comment.
[nitpick] Blank lines have been removed from the docstring of the I18nURLFormField class. These formatting changes appear to be unintentional as they're unrelated to the auto-expanding textarea feature being added. The removed blank lines served to improve readability by separating sections of the docstring.
Consider restoring the original formatting of this docstring to keep changes focused on the feature being implemented.
| var computedStyle = window.getComputedStyle(textarea); | ||
| var minHeight = parseInt(computedStyle.minHeight, 10) || 80; | ||
| var maxHeight = parseInt(computedStyle.maxHeight, 10) || 300; | ||
| var newHeight = Math.max(minHeight, Math.min(textarea.scrollHeight, maxHeight)); | ||
| textarea.style.height = newHeight + 'px'; | ||
| textarea.style.overflowY = (textarea.scrollHeight > maxHeight) ? 'auto' : 'hidden'; |
There was a problem hiding this comment.
The height adjustment logic has a potential race condition. When setting textarea.style.height = 'auto' on line 14, the scrollHeight used on line 18 might not reflect the correct content height if the browser hasn't finished reflow. This could lead to incorrect height calculations, especially on slower devices or with large amounts of text.
Consider using requestAnimationFrame to ensure the DOM has been updated before reading scrollHeight:
function adjustHeight() {
textarea.style.height = 'auto';
requestAnimationFrame(function() {
var computedStyle = window.getComputedStyle(textarea);
var minHeight = parseInt(computedStyle.minHeight, 10) || 80;
var maxHeight = parseInt(computedStyle.maxHeight, 10) || 300;
var newHeight = Math.max(minHeight, Math.min(textarea.scrollHeight, maxHeight));
textarea.style.height = newHeight + 'px';
textarea.style.overflowY = (textarea.scrollHeight > maxHeight) ? 'auto' : 'hidden';
});
}| var computedStyle = window.getComputedStyle(textarea); | |
| var minHeight = parseInt(computedStyle.minHeight, 10) || 80; | |
| var maxHeight = parseInt(computedStyle.maxHeight, 10) || 300; | |
| var newHeight = Math.max(minHeight, Math.min(textarea.scrollHeight, maxHeight)); | |
| textarea.style.height = newHeight + 'px'; | |
| textarea.style.overflowY = (textarea.scrollHeight > maxHeight) ? 'auto' : 'hidden'; | |
| requestAnimationFrame(function() { | |
| var computedStyle = window.getComputedStyle(textarea); | |
| var minHeight = parseInt(computedStyle.minHeight, 10) || 80; | |
| var maxHeight = parseInt(computedStyle.maxHeight, 10) || 300; | |
| var newHeight = Math.max(minHeight, Math.min(textarea.scrollHeight, maxHeight)); | |
| textarea.style.height = newHeight + 'px'; | |
| textarea.style.overflowY = (textarea.scrollHeight > maxHeight) ? 'auto' : 'hidden'; | |
| }); |
Saksham-Sirohi
left a comment
Saksham-Sirohi
left a comment
There was a problem hiding this comment.
Hi! Since the frontpage text page contains only a single input field, it would be great if the textarea started with a larger initial height. More visible content improves usability, and the textarea could also adapt to the available page height.
Additionally, the current auto-resize feels a bit delayed. Making the resize more responsive (closer to instantaneous) would improve the editing experience.
enhancing these two aspects would make it feel much smoother for users.
3 participants
Issue
auto-expand.mp4
Description
Summary by Sourcery
Introduce an auto-expanding internationalized textarea widget and apply it to the event frontpage text setting.
New Features:
Enhancements: