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.

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

Follow-up on Issue #530 #534

Closed
wants to merge 9 commits into from
Closed

Conversation

DMCarrajola
Copy link

This is a follow-up to issue #530 that I previously reported.

If you check the issue details, you'll see that the Save button is stuck due to an unnecessary input field. The core problem is that both input fields share the same name, question_id, which causes unintended validation conflicts.

Since I am not familiar with the entire scope of the project, I was hesitant to modify the ID of the correct input field, as I was concerned it might interfere with the database or other templates. While I searched through the project to confirm this, I couldn’t find any direct dependencies. However, as a beginner, I didn’t want to take any unnecessary risks.

To address this, I explored two possible solutions:

(Implemented in my fix): I changed the request method from pull/request to AJAX, incorporating local JavaScript to handle the process.
(Alternative approach): I considered auto-filling the "Add A Question" input with a placeholder (dummy) value, assuming that this information would be ignored. However, this approach carried potential risks of causing issues later on.
I apologize if my explanation isn’t entirely clear. As I mentioned earlier, I am still a beginner and have been using AI assistance, which sometimes prioritizes quick solutions over optimal ones.

Thanks for your patience!
Cheers! 🚀

Rmcd20 and others added 9 commits February 18, 2025 17:26
1
2
2
3
3
3
33
{% comment %} <span class="oh-radio__checkmark" ></span> NAO PRECISAMOS DE CHECKBOX NAS VIEWS {% endcomment %}
1
1
1
1
                  {% comment %} <span class="oh-radio__checkmark" ></span> NAO PRECISAMOS DE CHECKBOX NAS VIEWS apenas as respostas {% endcomment %}

The radio button is not needed in the response view as it causes confusion with min.js.
Hi, i opened that issue (issue horilla-opensource#530), and I am here showing a possible fix I found.

📌 Changes in question_all.html

Line 89: Replaced data-target with onclick to call JavaScript functions for handling the edit button’s expected behavior.
Lines 98-100: Changed {% trans %} to {% blocktrans %} to resolve a VSCode warning. (I didn't notice any functional difference, so feel free to remove it if unnecessary.)
Lines 104-105: Replaced data-id-save with onclick to ensure the save button triggers the required function.
Lines 115-156: Added JavaScript for handling the edit/save process. Not sure if this is the best place for it. If wanna change tell me where, im open to suggestions!

📌 Changes in views.py

Updated how edited data is passed to the backend using Ajax. This was the best solution I found.

🛠 General Notes

Got some help from AI, so if anything seems off, let me know! 😅
Open to feedback — hope this all makes sense!
Cheers!
@DMCarrajola
Copy link
Author

This isnt proper to be reviewed, sorry.

@DMCarrajola DMCarrajola deleted the DC branch February 26, 2025 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants