Direct Attachment Uploads#67
Conversation
gracewhitney
left a comment
gracewhitney
left a comment
There was a problem hiding this comment.
Should also add documentation of the new feature to the cookiecutter readme and the project readme
| <th>Name</th> | ||
| <th>Created By</th> | ||
| <th>Created On</th> | ||
| {% endraw %} |
There was a problem hiding this comment.
🥢 I'd avoid having endraw at a different indentation level than raw, for readability - i.e. I'd unindent this line. Consider keeping all cookiecutter directives at the top-level indentation to visually distinguish them from django template directives
| "{% if cookiecutter.django_react == 'disabled' %}config/webpack_loader.py{% endif %}", | ||
| "{% if cookiecutter.django_react == 'disabled' %}nwb.config.js{% endif %}", | ||
| "{% if cookiecutter.django_react == 'disabled' and cookiecutter.sass_bootstrap == 'disabled' %}package.json{% endif %}", | ||
| "{% if cookiecutter.django_react == 'disabled' and cookiecutter.sass_bootstrap == 'disabled' and cookiecutter.vue == 'disabled' %}package.json{% endif %}", |
There was a problem hiding this comment.
You can also add the vue static directory (which should probably be a subdirectory of a static root) to this, instead of adding the if vue to each individual vue file. Same for vite config.
Could also consider removing the whole app directory if reference_examples is disabled (would require associated settings changes etc)
| <a href="{% url 'sample-object' sample_object.id %}"> | ||
| <button class="btn btn-sm btn-secondary">View</button> | ||
| </a> | ||
| <a href="{% url 'sample-object-edit' sample_object.id %}"> | ||
| <button class="btn btn-sm btn-secondary">Edit</button> | ||
| </a> |
There was a problem hiding this comment.
| {% endraw %} | ||
| {%- if cookiecutter.crispy_forms == "enabled" %} | ||
| {%- raw %} | ||
| {% crispy form %} |
There was a problem hiding this comment.
This is a bit weird if crispy forms isn't enabled - should probably do an else with the built-in form tag, or remove the conditionals and add a readme note that crispy is required for this feature. I believe all other example forms are currently only present if crispy is enabled.
There was a problem hiding this comment.
I think this template is a bit misnamed - I'd expect a file of this name to be just the input itself. Consider renaming to include dashboard
There was a problem hiding this comment.
Why is this in directives?
| queryParams = {}, | ||
| ) => { | ||
| const doQuery = async () => { | ||
| return await get(url, options, headers, queryParams).then(async res => { |
There was a problem hiding this comment.
🥢 This has some weird mixes of async/await and callbacks which we've since cleaned up on DA - may want to also clean up here (we've also recently added retry behavior that could be worth using as a reference)
| "@babel/plugin-transform-react-jsx": "~7.16.7" | ||
| "@babel/plugin-transform-react-jsx": "~7.16.7", | ||
| {%- endif %} | ||
| {% if cookiecutter.vue == "enabled" -%} |
There was a problem hiding this comment.
uppy should be only if direct file upload is enabled
| django-extensions>=3.2 | ||
| requests | ||
| psycopg2 | ||
| whitenoise |
There was a problem hiding this comment.
remove (currently duplicated below, should be optional)
2 participants
This PR adds the attachments direct attachments upload feature (from zagaran/sample-django-app#48)