Fix: Limit Default Text Placeholders to Organizer-Selected Fields #772
Conversation
Reviewer's GuideThis PR refactors the PDF ticket editor to replace the single content placeholder with two dedicated dropdowns (event info and user info) featuring optional “other” fields, updates the editor.js methods to sample, load, and apply selections correctly (including a new default of “event_name”), adjusts the template to render and filter the dual dropdowns, and adds a standalone HTML test page for the new interaction. Sequence diagram for updating text area with dual dropdownssequenceDiagram
actor User
participant EventInfoDropdown as Event Info Dropdown
participant UserInfoDropdown as User Info Dropdown
participant EditorJS as editor.js
participant Canvas as PDF Canvas
User->>EventInfoDropdown: Selects an option
EventInfoDropdown->>EditorJS: Triggers change event
EditorJS->>UserInfoDropdown: Clears selection
EditorJS->>Canvas: Updates text area with selected event info
User->>UserInfoDropdown: Selects an option
UserInfoDropdown->>EditorJS: Triggers change event
EditorJS->>EventInfoDropdown: Clears selection
EditorJS->>Canvas: Updates text area with selected user info
User->>EventInfoDropdown: Selects 'Other'
EventInfoDropdown->>EditorJS: Triggers change event
EditorJS->>Canvas: Shows 'Other' textarea
User->>Canvas: Enters custom text
Canvas->>EditorJS: Triggers input event
EditorJS->>Canvas: Updates text area with custom text
Class diagram for updated editor.js text area handlingclassDiagram
class Editor {
+_get_text_sample(key)
+_load_pdf(dump)
+_update_toolbox_values()
+_update_values_from_toolbox()
+init()
}
Editor o-- EventInfoDropdown : uses
Editor o-- UserInfoDropdown : uses
class EventInfoDropdown {
+change()
+val()
}
class UserInfoDropdown {
+change()
+val()
}
Editor o-- TextAreaOther : manages
class TextAreaOther {
+show()
+hide()
+val()
}
File-Level Changes
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 @yaxit24 - I've reviewed your changes - here's some feedback:
- There's a lot of duplicated logic around toggling and clearing the two dropdowns; consider extracting that into reusable helper functions to reduce repetition and improve maintainability.
- The manual
test_dual_dropdown.htmlfile lives at the project root but isn't part of the automated suite; consider moving it into a dedicated test directory or converting it into an automated JS test. - The
initmethod now contains many event handler attachments; you might want to group those into a separate private method (e.g._bindDualDropdownEvents) to keepinitconcise.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There's a lot of duplicated logic around toggling and clearing the two dropdowns; consider extracting that into reusable helper functions to reduce repetition and improve maintainability.
- The manual `test_dual_dropdown.html` file lives at the project root but isn't part of the automated suite; consider moving it into a dedicated test directory or converting it into an automated JS test.
- The `init` method now contains many event handler attachments; you might want to group those into a separate private method (e.g. `_bindDualDropdownEvents`) to keep `init` concise.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
I have Reduced Duplicated Logic, Organized Event Handler Attachments
|
All the changes are made as sugegsted by the Sourcery-ai. |
yaxit24
changed the title
There was a problem hiding this comment.
This resolves the issue #769 by categorizing the placeholders into two distinct categories. However, it does not actually limit the placeholders. The original issue #192 aimed to achieve the functionality from eventyay.com, where the placeholders for the badge were determined by the fields selected for the Attendee Form. This ensured that the fields in the badge would always be available and would not end up being empty.
Choosing Attendee Form fields from eventyay.com:
Choosing Fields for Badges in eventyay.com:
|
what i think the issue of the limiting the placeholders can be solved here by simply limiting them here, keeping only those placeholders which are utilised most, or removing those which are irrelevant for badges in events. |
|
No, that may not be the correct approach. Some events may require specific fields, while others may not. It is not advisable to base the editor on the most utilized field, but rather on the information available for the event. For instance, if an event does not have a “Seat Number” field, it is not necessary to include it in the editor. Instead, the editor should be dynamic and display the fields of data available for the event. |
|
thankyou @Sak1012, @hpdang and @mariobehling what are reviews ? |
|
Please follow the suggestions of Srivatsav. There is no reason to artificially limit the fields. There are many use cases where we need fields that have been filled in. Thanks |
Fixes: #769
Screen.Recording.2025-06-30.at.12.32.31.PM.mov
Summary by Sourcery
Implement separate Event Info and User Info placeholder dropdowns and adjust editor logic to handle dual dropdowns with "Other" inputs, restricting default placeholders to configured fields.
New Features:
Bug Fixes:
Enhancements:
Tests: