Initial DSC & Django update #660

tymees · 2024-04-22T12:52:14Z

This PR lays the foundations to built V4 upon.

Note: this PR targets the major/4 branch for now. This is a fork of develop made at the same time as the source branch, to have a clear diff. New changes in develop will need to be merged into there as well.

It's a rather large PR; I've tried to make proper commits for individual changes, to help reviewing. I've also added some comments over adding them here, such that the comment is located near the relevant code.
Also, please ignore the changes in the docs folder. Those are caused by my bulk-refactors.

Highlights:

Update to Django 4.2
- I think I got this all sorted, but there might still be some problems lurking in the later form pages
Updated to DSC 3
- New app-specific base template (and moved all pages to it)
- New form-specific base template (and moved the first form + WMO forms to it)
- New home-page design. (Needs sign-off from desiree)
- Initial automatic CSS migration pass (renaming classes to their new equivalents mostly)
- Ported form-JS utils to new form (non-table-based) layout

Still todo:
Well, a lot. A non-exhaustive list:

Layout polishing pass
All other form pages need to be converted to the new form-base-template
- In addition, some pages might need to be split up to be less intimidating
All other pages need to be (re-)reviewed.
A proper stepper implementation
New overview pages using UU-List. (Ideally we'd cut down on the amount of those pages in favour of filters)
Everything else in https://github.com/DH-IT-Portal-Development/ethics/milestone/3
And probably stuff I'm forgetting

Mostly ensuring the app starts up, the app is still thoroughly FUBAR

Switches to new app-specific base-template, and initial refactor to new bootstrap CSS classes

File is still there to avoid errors on non-migrated form-pages. To be removed later

The old file is added bak-file, so we can pull out relevant sections when needed

Yes, totally unnecessary, but I like it

tymees · 2024-04-22T15:24:54Z

main/static/main/style.css

I've moved the original contents to style.css.bak. I did this because I want us to manually review anything we need from there. A lot of it is unneeded now, and other stuff might be better implemented using existing classes from BS.

tymees · 2024-04-22T15:27:17Z

main/templates/base/fetc_base.html

This is the new base template for ethics, to be used by all pages either directly or indirectly. It mostly implements the header styling, leaving the main content styling up to the DSC provided template.

Note: the current header isn't quite finished, it still needs a polishing pass. However, that would delay this PR, and it's functional enough for now

tymees · 2024-04-22T15:30:37Z

main/templates/base/fetc_form_base.html

This is the more controversial one of the two new base templates. This template is designed to be used by most 'form'-templates (with 'form templates' I only mean the ones part of the actual application. We have other forms in the app, like 'copy', that should just the normal base template. (Altough, we could create a more generic form-template that handles for FormView-derivatives, that handles the HTML for forms.)

Note: some of our application-form-pages are a bit special, and might need a custom template instead of this 'automagic' template.

This template sets a pre-set layout, providing specific blocks to override to put content in said content. See the following image for a visual overview of the available blocks and where they are:

Notes:

The sizes of the blocks were hardcoded by me, they scale dynamically to the content (vertically)

All blocks can be left empty. In the pages I've conferted I've used pre-form to add the page-title

The stepper is a block, but ideally that block would never need to be overwritten by any implementing page.

The're a block form-css-classes not visually present in the image above. This is a block one can use to add extra css-classes to the uu-form element. (For example uu-form-no-gap, or uu-form-no-help).

Awesome work, and great image.

I think the pre-form, form-buttons, and post-form should be easily splittable into a their main content and an aside, as with the actual form.

This makes a more sense stylistically, for example to align the buttons to the main form body. This also allows for an aside to the pre-form intro text, for example with some "handige links" like we do in procreg.

Of course this is already possible if you code it yourself or reuse the classes. However, it would be nice to have it supported by default everywhere in the form by having separate pre-form-content and pre-form-aside blocks you could use right away.

And then a parent pre-form block which gives you the full width if you replace it.

That's a great suggestion! As it's quite easy to implement, I've done so:

Some notes:

As you suggested, these new blocks are childs of the existing pre/post-form blocks, so you can still override those as before.

I called pre-form-content pre-form-text instead, as that mirrors the class .uu-form provides for this usecase

I didn't split up form-buttons into an aside, as I don't see a usecase where that makes sense to actually use. I did change the HTML to use the same text classes as the new blocks, such that the buttons are aligned with the form as you suggested.

With this buttons-change, we probably want make those form buttons smaller. Currently I think it's a bit claustrophobic

tymees · 2024-04-22T15:48:17Z

main/templates/base/form_buttons.html

Controversially, I removed all buttons aside from next/prev. Mostly because the new stepper should be the new best-way to nagivate to things.

tymees · 2024-04-22T15:48:55Z

main/templates/base/navigation.html

This was the old stepper. It's still referenced on a lot of pages, so I just made an empty file for now

tymees · 2024-04-22T15:58:54Z

main/templates/base/stepper.html

This is a exceptionally terrible implementation of the new stepper on top of the old stepper code; it's mostly for show.

In a different issue, this should be replaced with a properly working one (ideally with status indication, and better hierarchy).

tymees · 2024-04-22T16:04:00Z

main/views.py

@@ -57,8 +57,58 @@ def get_queryset(self):
        return self.model.objects.filter(not_after__gt=now, not_before__lt=now)


-class HomeView(LoginRequiredMixin, _SystemMessageView):
+class SeasonalCoverImageMixin:


Okay so this addition is 100% unnecessary. I just wanted to have a bit of fun during a boring meeting.

It adds a seasonal-appropiate cover-image automatically.

The winter and autumn covers are the ones we already had, the spring and summer ones are new.

Note 1: I made the old 'summer' version the 'autumn' one. Mostly because it had people in winter-coats in it, so it wasn't really summer.
Note 2: I found out that our attributions we had before were wrong! We listed the person who added the image to the image-bank, not the actual photographer themselves. The ones listed here now are correct.

tymees · 2024-04-22T16:08:04Z

main/views.py

+    def form_valid(self, form):
        messages.success(self.request, self.success_message)
-        return super(DeleteView, self).delete(request, *args, **kwargs)
+        return super(DeleteView, self).form_valid(form)


Django 4 changed how deleteviews worked. For those wondering where this and other similar changes comes from.

Wth? Was djlint unhappy with its own output?!

Okay, this should be all of them now. RIGHT?!

EdoStorm96

Wow, very good work :) I mainly looked at what you changed, not so much at what stayed the same. And i did some, but non-exhaustive testing. It all looks good and consistent. The one thing I think you overlooked is to remove {% load uil_filters %} from a few templates. This caused an error for me at one point in testing.

I also quite like the philosophy of the new fetc_form_base template. I think the places you've used the new boxes, show the potential of this nicely.

All in all, I'm happy and excited to continue working on this!

EdoStorm96 · 2024-04-25T08:56:19Z

reviews/templates/reviews/committee_members_workload.html

Maybe add a btn class to the submit button on this page. My bad, i think :p

Yeah, this is one of the files only touched by my automatic refactoring runs; and I didn't do a check for naked submit buttons. (There are probably more?)

And not really your fault; the previous CSS handled that automatically for you, the new one doesn't.

tymees · 2024-04-25T09:45:03Z

Thanks for the review!

I've added a commit that refactors all uses of uil_filters.

There are probably more of these 'well you could've done that in bulk' problems still living in other places; I've only done those I was aware of, and those I found during my work on the pages I gave individual attention.

Normally we go with the 'one approval is enough' strategy, but I really want @miggol's review before merging, as it's quite an impactfull PR.

miggol · 2024-04-30T13:32:08Z

main/templates/main/landing.html

Landing

The middle-aligned navigation bar up top looks weird. Like it's crying for help. I think aligning it to the right/flex-end would help. I expect this would look find for most other pages with more nav items.

Yeah honestly I think the menu should just be empty if you're not logged in, especially if we add the footer from index.html here as well.

It's middle-aligned because procreg is middle-aligned, and it's a bit annoying to fix that manually for this one case

miggol · 2024-04-30T13:33:25Z

main/templates/main/landing.html

I would like to add the footer from index.html to this page. The documents links might not be as relevant, but all the support addresses are.

Good idea!

However, it's (sadly) not as simple as moving some HTML around. landing.html has some extra JS to protect against spambots scanning mailto: links, which we'd have to re-introduce. I'd rather create a separate issue for this, to implement that correctly.

miggol · 2024-04-30T14:52:28Z

main/templates/base/fetc_form_base.html

Awesome work, and great image.

I think the pre-form, form-buttons, and post-form should be easily splittable into a their main content and an aside, as with the actual form.

This makes a more sense stylistically, for example to align the buttons to the main form body. This also allows for an aside to the pre-form intro text, for example with some "handige links" like we do in procreg.

Of course this is already possible if you code it yourself or reuse the classes. However, it would be nice to have it supported by default everywhere in the form by having separate pre-form-content and pre-form-aside blocks you could use right away.

And then a parent pre-form block which gives you the full width if you replace it.

miggol

Alright, I've been browsing for a while and overall I'm very impressed. Most things are simply working.

There's some stuff still missing but I'm sure we'll get to it soon enough in the django-4 branch.

Ugh

tymees added 18 commits April 15, 2024 17:55

feat: initial Django 4.2 update

9fb113f

feat: initial DSC3 updates

f2bf87e

Mostly ensuring the app starts up, the app is still thoroughly FUBAR

fix: corrected method signature

16f7496

feat: added app-specific base-template

6963e46

refactor: bulk update templates

8e95dfc

Switches to new app-specific base-template, and initial refactor to new bootstrap CSS classes

feat: initial new homepage design

2df9104

refactor: remove unused menu entries

eee1476

refactor: port JS form functions to new form code

627fc2b

refactor: remove unused template

00a7f4e

refactor: remove old stepper

71d3a93

File is still there to avoid errors on non-migrated form-pages. To be removed later

feat: added new form-specific base template

2e02f60

refactor: empty out app-specific css sheet

5e4b00d

The old file is added bak-file, so we can pull out relevant sections when needed

feat: use TemplatedModelForm as base for conditional_form.py

41fdcdf

refactor: move proposal_form.html to new form-base

291f574

refactor: move wmo_application.html to new form-base

b1c03af

refactor: move wmo_form.html to new form-base

1cdd7f8

refactor: move wmo_form.html to new form-base

54316e1

feat: seasonal banner

edc6a30

Yes, totally unnecessary, but I like it

tymees marked this pull request as draft April 22, 2024 12:52

tymees added 3 commits April 22, 2024 15:08

refactor: modern app load-order

ec9fc1b

refactor: use new url prefix for core

bb46fe6

refactor: better cols spacing method

5208bc2

tymees commented Apr 22, 2024

View reviewed changes

tymees added 3 commits April 23, 2024 12:49

style: black & djlint

8e19b1c

chore: update translations

5f6627b

style: djlint v2

cf5469d

Wth? Was djlint unhappy with its own output?!

tymees marked this pull request as ready for review April 23, 2024 11:38

tymees requested review from miggol and EdoStorm96 April 23, 2024 11:39

fix: fix safestring issues

c94db97

Okay, this should be all of them now. RIGHT?!

EdoStorm96 approved these changes Apr 25, 2024

View reviewed changes

refactor: uil_filters -> cdh_filters

29ad241

miggol reviewed Apr 30, 2024

View reviewed changes

miggol approved these changes Apr 30, 2024

View reviewed changes

tymees added 4 commits April 30, 2024 18:28

refactor: remove redundant <table>

17f5dfe

feat: extra form-blocks to override

473f11f

fix: commit the _right_ file

8a4e9b5

Ugh

style: djlint

f6d2af7

tymees merged commit 839a349 into major/4 May 6, 2024
3 checks passed

tymees deleted the update/django_dsc branch May 6, 2024 15:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial DSC & Django update #660

Initial DSC & Django update #660

tymees commented Apr 22, 2024 •

edited

Loading

tymees Apr 22, 2024

tymees Apr 22, 2024

tymees Apr 22, 2024 •

edited

Loading

miggol Apr 30, 2024

tymees Apr 30, 2024

tymees Apr 22, 2024

tymees Apr 22, 2024

tymees Apr 22, 2024

tymees Apr 22, 2024

tymees Apr 22, 2024

EdoStorm96 left a comment

EdoStorm96 Apr 25, 2024

tymees Apr 25, 2024

tymees commented Apr 25, 2024

miggol Apr 30, 2024

tymees Apr 30, 2024

miggol Apr 30, 2024

tymees Apr 30, 2024

miggol Apr 30, 2024

miggol left a comment

Initial DSC & Django update #660

Initial DSC & Django update #660

Conversation

tymees commented Apr 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tymees Apr 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EdoStorm96 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tymees commented Apr 25, 2024

Choose a reason for hiding this comment

Landing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miggol left a comment

Choose a reason for hiding this comment

tymees commented Apr 22, 2024 •

edited

Loading

tymees Apr 22, 2024 •

edited

Loading