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

Update modals to use htmx/alpine #4053

Merged
merged 17 commits into from
Aug 8, 2024
Merged

Update modals to use htmx/alpine #4053

merged 17 commits into from
Aug 8, 2024

Conversation

theskumar
Copy link
Member

@theskumar theskumar commented Jul 15, 2024 •
edited
Loading

  • Delete modal
  • Update status modal
  • update reviewer
  • Archive/unarchive modal and view/logic
  • Create reminder modal
  • update reminder block design
  • Add modal title component
  • Django messages as toast for htmx request
  • update lead modal response
  • meta term modal
  • new project modal
  • partner update modal
  • Adds new htmx based handler for django-message-framework and display them as toast

Fixes #3391

Test Steps

  • Check above update modals work properly

@theskumar theskumar self-assigned this Jul 15, 2024
@theskumar theskumar mentioned this pull request Jul 15, 2024
Closed
4 tasks
@theskumar theskumar force-pushed the enhancement/gh-3991-htmx-dialog-alt branch from 3aeec1e to 535088e Compare July 15, 2024 07:24
frjo
frjo requested changes Jul 15, 2024
View reviewed changes
Copy link
Contributor

@frjo frjo left a comment

Choose a reason for hiding this comment

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

Really nice overall!

I do not think the modals should close when clicking the background, omnly when a button is pressed.

If you e.g. open the Reviewers modal pick some reviewers and then click outside the modal you loose your selection.

@theskumar theskumar force-pushed the enhancement/gh-3991-htmx-dialog-alt branch from 535088e to 385ff74 Compare July 15, 2024 07:39
@theskumar
Copy link
Member Author

theskumar commented Jul 15, 2024 •
edited
Loading

I do not think the modals should close when clicking the background, omnly when a button is pressed.

Yes. that can be made possible.

Edit: fixed in 850a50e

@theskumar theskumar force-pushed the enhancement/gh-3991-htmx-dialog-alt branch 5 times, most recently from a23c76e to 82429c3 Compare July 22, 2024 07:59
@theskumar theskumar marked this pull request as ready for review July 22, 2024 08:00
@theskumar
Copy link
Member Author

This is good to review now. I've updated/fixed the meta terms display and forms. It should correctly fill the values in edit now.

Screenshot 2024-07-22 at 8  01 31@2x

@theskumar
Copy link
Member Author

I'll fix over the failing test, the modal for the batch actions are not update, at the table view will be replaced soon in my next PR.

@frjo frjo added Status: Needs testing Tickets that need testing/qa Type: Feature This is something new (not an enhancement of an existing thing). Type: Minor Minor change, used in release drafter Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Jul 24, 2024
@frjo
Copy link
Contributor

frjo commented Jul 24, 2024

@theskumar Some minor issues with test. Seem to be looking for class names so might be an easy fix.

@theskumar
Copy link
Member Author

@frjo Updated test cases.

@theskumar theskumar force-pushed the enhancement/gh-3991-htmx-dialog-alt branch from 00678ae to 5258d4c Compare July 25, 2024 05:33
@theskumar
Copy link
Member Author

rebased with latest main.

@theskumar theskumar force-pushed the enhancement/gh-3991-htmx-dialog-alt branch from 8bf9418 to 6f2c9dd Compare July 25, 2024 05:40
@frjo
Copy link
Contributor

frjo commented Jul 29, 2024

Performance improvements:

  1. Fewer db queries, 30-40% less.
  2. Lower CPU time: 20-30% improvement.
  3. Page load time: 10-15 % improvement.

@theskumar theskumar force-pushed the enhancement/gh-3991-htmx-dialog-alt branch from 6f2c9dd to fc6eecc Compare August 8, 2024 07:33
@theskumar theskumar mentioned this pull request Aug 8, 2024
Open
6 tasks
sandeepsajan0 and others added 17 commits August 8, 2024 13:42
@sandeepsajan0 @theskumar
Use Dialog box inplace of fancybox modal, with htmx and alpine(only f…
0253f15 
…or lead update yet)

Use htmx and dialog for archive and unarchive modals

Use htmx and dialog for project creation

Use htmx and dialog for create reminder form

Use htmx and dialog for submission progress/status update

Fix requirements and progress button code

Use htmx and dialog box for reviewers update modal

Use htmx and dialog box for partners update modals

Use htmx and dialog box for meta terms

Fix test for htmx and dialog box modals

Replace updated lead and partners directly to element in html instead of reloading the page

Avoid full page reload on reviewers update

Avoid page reload for reminder and meta terms

Remove Select2Widget for reviewers and partners, add defer to alpine dialog

Use Dialog modal instead of html-only dialog, fix dialog closing and UI shifting issue

Use dialog modal for all other dialogs to avoid UI shift and closing issue

Change atag to button tag for Modal trigger buttons

Add @fylgja/alpinejs-dialog as npm package
@theskumar
New dialog implementation
22f0ce5
@theskumar
Delete modal
8066962
@theskumar
Update status modal
dabc93b
@theskumar
update reviewer
6e48709
@theskumar
Archive/unarchive modal
0c35a9c
@theskumar
Create reminder modal
2667118
@theskumar
update reminder block design
723d12a
@theskumar
Add modal title component
08f1120
@theskumar
Django messages as toast for htmx request
fa910c1
@theskumar
update lead modal response
2b9d8b2 
remove dependencies
@theskumar
Do no close modal if clicked outside
0822bdd
@theskumar
update modal title, create project, ...
95ee023
@theskumar
Finish lead update form
10935b7
@theskumar
update meta terms modal and remove mobile actions
8a81c6b 
update how meta terms are rendered in submission detail

translation markers, remove meta term api
@theskumar
update test cases to use data-testid
606ddaa
@theskumar
Add i18n
82967c7
@theskumar theskumar force-pushed the enhancement/gh-3991-htmx-dialog-alt branch from fc6eecc to 82967c7 Compare August 8, 2024 08:13
@wes-otf
Copy link
Contributor

wes-otf commented Aug 8, 2024 •
edited
Loading

This is fantastic! Testing went really well and everything looks much more slick. The only very minor issue I saw was the reminder toast showed twice upon creation but looking at the code that may be a reminders specific issue.
Screenshot 2024-08-08 at 10 46 29

@frjo if it's determined that requires a separate issue this is ready to be merged

@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 Aug 8, 2024
@frjo frjo merged commit ef47125 into main Aug 8, 2024
10 checks passed
@theskumar theskumar deleted the enhancement/gh-3991-htmx-dialog-alt branch August 9, 2024 12:14
@theskumar
Copy link
Member Author

This is fantastic! Testing went really well and everything looks much more slick. The only very minor issue I saw was the reminder toast showed twice upon creation but looking at the code that may be a reminders specific issue.

This is probably because it's specifically being asked to display and other one is via the django messages framework through the activity feed route. I think it should be quick and safe to remove one of them.

theskumar added a commit that referenced this pull request Aug 9, 2024
@theskumar
Merge remote-tracking branch 'origin/main' into submission-table-view
693eb43 
* origin/main:
  Notification upon profile update (#3970)
  Fix error in View All when no reviewer role image (#4068)
  Remove tasks from task list (#4040)
  Update modals to use htmx/alpine (#4053)
  Fix: Task is not removed for staff while project moved to next stage (#4026)
  Speed & appearance improvements to the results page (#4043)
@frjo
Copy link
Contributor

frjo commented Aug 9, 2024

Added new issue for reminder alter bug. #4082

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frjo frjo frjo approved these changes

@wes-otf wes-otf wes-otf approved these changes

@sandeepsajan0 sandeepsajan0 Awaiting requested review from sandeepsajan0

Assignees

@theskumar theskumar

Labels
Status: Tested - approved for live ✅ Type: Feature This is something new (not an enhancement of an existing thing). Type: Minor Minor change, used in release drafter
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Convert all dialogs/modals to use the <dialog> tag and htmx instead of fancybox lib
4 participants
@theskumar @frjo @wes-otf @sandeepsajan0