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

Plate Diagram #8

Merged
merged 5 commits into from
Mar 8, 2025
Merged

Plate Diagram #8

merged 5 commits into from
Mar 8, 2025

Conversation

covracer
Copy link
Contributor

@covracer covracer commented Mar 4, 2025

Background and Links

  • Diagrams/maps/tables of plate contents should help people review them for correctness
  • I experimented with an SVG plate diagram, but the drawn circles and text weren't vertically aligned

Changes and Testing

  • Bump tooling
  • Add backend-generated table HTML
    Screenshot from 2025-03-02 09-28-51

Questions and Followup

  • How to "progressively", in the spirit of a Progressive Web Application (PWA) I think, make the "Bottom right prefix" and "Diagram" update with preview values in real time as their inputs change? Some similar seeming code exists in Django for "prepopulated_fields", implemented in jquery.
  • Other related Django functionality I've discovered is MultiValueField. Should bottom_row, right_column, and prefix be demoted from first-class models to properties, annotations, or GeneratedFields, and the compressed bottom_right_prefix representation of them become the sole admin interface representation, as a MultiValueField?
  • Alpine.js looks really nice but I don't see a lot of Django admin integration examples. In particular, I'm not sure how to add x-data to a big enough <div> tag without pretty elaborate template overrides.

Copy link

@jamesbraza jamesbraza left a comment

Choose a reason for hiding this comment

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

Nice

Comment on lines +19 to +20
# mypy last because it's so slow
- id: mypy

Choose a reason for hiding this comment

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

I have the same conclusions in my own pre-commit toolchains, fun to see us converging

Comment on lines +149 to +151
with raises(AttributeError):
Container().get_source_expressions() # type: ignore[misc,operator]
with raises(AttributeError):

Choose a reason for hiding this comment

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

I like to use match kwarg to tighten up these assertions further, I have seen drift in errors with same type happen before

Also, feel free to ignore this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@covracer covracer merged commit 9ea10e3 into main Mar 8, 2025
2 checks passed
@covracer covracer deleted the toolbump branch March 8, 2025 12:08
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