Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

#3906 un-nest labels and inputs in themes #3908

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

StephDriver
Copy link
Contributor

all file search for labels which nested their input and only two were found, both in theme templates marked as depreciated in 1.5.1.

Labels elsewhere looked un-nested, and hence avoiding this issue.

Closes #3906

Copy link
Member

@ajrbyers ajrbyers left a comment

Choose a reason for hiding this comment

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

This is okay, does the display remain the same after the change?

@StephDriver StephDriver requested a review from ajrbyers March 5, 2024 15:02
Copy link
Member

@joemull joemull left a comment

Choose a reason for hiding this comment

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

Just thought I'd mention this template is no longer used, to my knowledge. I added this deprecation note recently when I switched this interface to using a class-based view:

https://github.com/BirkbeckCTP/janeway/blob/9fcbbe87b917bf3c4ce1d64b85d579f7c0634069/src/themes/OLH/templates/elements/journal/article_filter_form.html#L1

Maybe I should have just deleted the template? I wasn't confident doing so without more front-end tests in place.

@ajrbyers
Copy link
Member

ajrbyers commented Mar 14, 2024

Just thought I'd mention this template is no longer used, to my knowledge. I added this deprecation note recently when I switched this interface to using a class-based view:

https://github.com/BirkbeckCTP/janeway/blob/9fcbbe87b917bf3c4ce1d64b85d579f7c0634069/src/themes/OLH/templates/elements/journal/article_filter_form.html#L1

Maybe I should have just deleted the template? I wasn't confident doing so without more front-end tests in place.

I think its safe to remove it in 1.6. Any break in rendering of the article list view will be picked up by https://github.com/BirkbeckCTP/janeway/blob/9fcbbe87b917bf3c4ce1d64b85d579f7c0634069/src/journal/tests/test_journal_frontend.py#L98-L112.

@joemull
Copy link
Member

joemull commented Mar 14, 2024

@ajrbyers Brilliant! I've added an issue for it.

@joemull joemull merged commit 343064b into master Mar 14, 2024
1 check failed
@joemull joemull deleted the 3906-bugfix_form_field_missing_label branch March 14, 2024 09:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

form field missing a label
3 participants