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

added anthem translation field as of enhancement issue #21 #54

Merged
merged 5 commits into from
Sep 7, 2024

Conversation

MonEstCha
Copy link
Collaborator

No description provided.

@jftsang
Copy link
Owner

jftsang commented Aug 2, 2024

added you as a collaborator

forms.py Outdated
Comment on lines 42 to 44
anthem_title = StringField('Anthem')
anthem_composer = StringField('Anthem composer')
anthem_lyrics = StringField('Anthem lyrics', widget=TextArea())
anthem_translation = SelectField('Anthem Translation', choices=translations)
Copy link
Owner

Choose a reason for hiding this comment

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

I don't know if Flask allows nesting of forms but it might be nice to group these together

models.py Outdated
@@ -438,13 +440,14 @@ def from_form(cls, form: 'PewSheetForm') -> 'Service':
else:
secondary_feasts = []

if form.anthem_title.data or form.anthem_composer.data or form.anthem_lyrics.data:
if form.anthem_title.data or form.anthem_composer.data or form.anthem_lyrics.data or form.anthem_translation.data:
Copy link
Owner

Choose a reason for hiding this comment

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

this would be simplified if we could group stuff together

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, I will give it a try!

@MonEstCha
Copy link
Collaborator Author

@jftsang, could you review my pull request, please?

@jftsang jftsang merged commit e495561 into jftsang:main Sep 7, 2024
3 checks passed
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