-
Notifications
You must be signed in to change notification settings - Fork 174
fix: reuse request template rather than duplicating #3189
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
base: master
Are you sure you want to change the base?
Conversation
zzacharo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@carlinmack this looks good to me, do you still work on it?
|
nope this is ready :) |
| {% from "invenio_requests/macros/request_header.html" import inclusion_request_header %} | ||
|
|
||
| {% set title = invenio_request.title %} | ||
| {% set base_template = admin_base_template %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice thought for reusing templates - just a little warning, you might be in trouble though to provide all the context needed for the parent template to work, especially when making changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion, to be tested if it wasn't done yet: all the request views
sakshamarora1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
❤️ Thank you for your contribution!
Description
Checklist
Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:
Frontend
Reminder
By using GitHub, you have already agreed to the GitHub’s Terms of Service including that: