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.

Sign up for GitHub

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

Jump to bottom

More clean up in how we load js and css files #4287

Merged
merged 13 commits into from
Jan 10, 2025
Merged

More clean up in how we load js and css files #4287

merged 13 commits into from
Jan 10, 2025

Conversation

frjo
Copy link
Member

@frjo frjo commented Dec 17, 2024 •
edited
Loading

  • Remove unused document-category.js.
  • Replaced toggle-payment-block.js with alpine code.
  • Load all *media.js|css that needs jQuery in block extra or directly in base.html. Move jQuery back to the bottom of the page.
  • Merge application-form-links-new-window.js in to application-form.js.
  • Load comment_form.media on submission and project detail templates directly.
  • Remove outdated block extras and move block extra css just above block extra js in templates.

Test Steps

  • Test all roles dashboards so all needed js and css gets loaded.
  • Test submission and project pages so all needed js and css gets loaded.
  • Test that all file upload fields, especially in htmx dialog, work as they should.

@frjo frjo added Type: Maintenance Type: Patch Mini change, used in release drafter labels Dec 17, 2024
@frjo frjo force-pushed the maintenance/clean-up-js-files branch from b24d254 to 8863a2e Compare December 17, 2024 19:06
@frjo frjo added Status: Needs testing Tickets that need testing/qa Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Dec 18, 2024
@wes-otf
Copy link
Contributor

wes-otf commented Dec 19, 2024

I didn't see any issues in my testing, seems pretty good!

@wes-otf wes-otf added Status: Tested - approved for live ✅ and removed Status: Needs testing Tickets that need testing/qa Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Dec 19, 2024
@frjo
Copy link
Member Author

frjo commented Dec 20, 2024

As @theskumar predicted this PR breaks file uploads in htmx dialogs.

@frjo frjo force-pushed the maintenance/clean-up-js-files branch from 8863a2e to 7c64842 Compare December 20, 2024 08:10
@frjo frjo changed the title More clean up in how we load js files More clean up in how we load js and css files Dec 20, 2024
@frjo frjo requested a review from theskumar December 20, 2024 08:41
@frjo frjo force-pushed the maintenance/clean-up-js-files branch 2 times, most recently from 3259aab to c982836 Compare January 3, 2025 12:50
@frjo frjo force-pushed the maintenance/clean-up-js-files branch from c982836 to b0d30ac Compare January 7, 2025 08:47
@frjo frjo added Status: Needs testing Tickets that need testing/qa Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Jan 7, 2025
frjo
frjo commented Jan 9, 2025
View reviewed changes
hypha/static_src/javascript/file-uploads.js
if (closestFormGroup) {
closestFormGroup.style.display = "none";
}
});
Copy link
Member Author

@frjo frjo Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not think this part is needed anymore. Did not find a place where it had any effect. Let it be for now since we are changing so many other things.

frjo
frjo commented Jan 9, 2025
View reviewed changes
hypha/static_src/javascript/file-uploads.js
@@ -1,26 +1,30 @@
jQuery(function ($) {
// We use htmx.onLoad() so it will initilise file uploads in htmx dialogs.
htmx.onLoad(function () {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really neat function!

@wes-otf wes-otf added Status: Tested - approved for live ✅ and removed Status: Needs testing Tickets that need testing/qa Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Jan 9, 2025
@wes-otf
Copy link
Contributor

wes-otf commented Jan 9, 2025

was using all dialogs quite a bit today and didn't see any issues jumping out at me so I think all is good unless there was something else specific you wanted me to look at @frjo

@frjo frjo merged commit a972eea into main Jan 10, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theskumar theskumar Awaiting requested review from theskumar

Assignees
No one assigned
Labels
Status: Tested - approved for live ✅ Type: Maintenance Type: Patch Mini change, used in release drafter
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@frjo @wes-otf @theskumar