Skip to content

Allow italics in a variety of content fields#4153

Merged
ajrbyers merged 17 commits into
masterfrom
4097-additional_html_display
Aug 29, 2024
Merged

Allow italics in a variety of content fields#4153
ajrbyers merged 17 commits into
masterfrom
4097-additional_html_display

Conversation

@joemull

@joemull joemull commented May 1, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

This branch adds support for italics in a variety of content fields, especially those that might include journal titles or other titles of works that need italicization.

We can do this more easily and safely now thanks to safer storage of HTML in the database introduced in 1.5.1.

Screenshot from 2024-05-01 16-01-25
Screenshot from 2024-05-01 15-59-57
Screenshot from 2024-05-01 15-59-30
Screenshot from 2024-05-01 15-58-55
Screenshot from 2024-05-01 15-58-44

For the copyright text, I have opted to avoid putting the journal name altogether. This will require a retranslation of that string but I believe it is worth it because the existing translation is not ideal, because it's a partial sentence (excludes the journal name) and does not include punctuation as part of the translated string (the colon).
image

Closes #4097.

@joemull joemull requested a review from ajrbyers May 1, 2024 15:13
@joemull joemull marked this pull request as draft May 1, 2024 15:16
@joemull

joemull commented May 1, 2024

Copy link
Copy Markdown
Member Author

Oh. Not quite ready. Problem with migrations.

@joemull joemull self-assigned this May 1, 2024
@joemull joemull marked this pull request as ready for review May 7, 2024 13:08
@joemull joemull assigned ajrbyers and unassigned joemull May 7, 2024
@joemull joemull mentioned this pull request May 17, 2024
Draft
@joemull

joemull commented May 24, 2024

Copy link
Copy Markdown
Member Author

@ajrbyers I just wanted to make sure you saw this one is ready for review now.

ajrbyers
ajrbyers reviewed May 24, 2024
View reviewed changes

@ajrbyers ajrbyers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've just given this a once over and before I test it out I wondered if there was a reason why the Frozen Author records had all been convered to bleach fields?

@joemull joemull removed their assignment May 24, 2024
@joemull

joemull commented May 24, 2024

Copy link
Copy Markdown
Member Author

I've just given this a once over and before I test it out I wondered if there was a reason why the Frozen Author records had all been convered to bleach fields?

Ah, yes it's because they get used in the carousel subtitle. We said we wanted to support italics in carousel text.

ajrbyers
ajrbyers approved these changes May 28, 2024
View reviewed changes

@ajrbyers ajrbyers left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've added a couple of comments/questions but this looks great.

Comment thread src/core/homepage_elements/html/views.py Outdated
Comment thread src/journal/migrations/0066_issue_type_bleach_20240507_1359.py
Comment thread src/submission/migrations/0080_frozen_author_bleach_20240507_1350.py
@ajrbyers ajrbyers assigned joemull and unassigned ajrbyers May 28, 2024
@joemull joemull assigned mauromsl and unassigned joemull May 29, 2024
@joemull joemull requested a review from mauromsl May 29, 2024 12:10
@ajrbyers

ajrbyers commented Jun 4, 2024

Copy link
Copy Markdown
Member

@joemull just FYI this PR now has a conflict.

@joemull joemull force-pushed the 4097-additional_html_display branch from d2a84ff to e885510 Compare June 5, 2024 04:16
@joemull joemull requested a review from ajrbyers June 5, 2024 04:21
@joemull

joemull commented Jun 5, 2024

Copy link
Copy Markdown
Member Author

@ajrbyers After rebasing I ran tests and picked up several things that needed to be addressed:

  1. Utilities like importers trying to save None to affected fields on FrozenAuthor and IssueType
  2. Fields from the Account model that allow null=True being directly saved to FrozenAuthor

I think this calls for standardizing the corresponding Account model fields, so I've done that. What do you think? Re-requesting review since it's a significant enough change.

@joemull joemull assigned ajrbyers and unassigned mauromsl and joemull Jun 5, 2024
ajrbyers
ajrbyers approved these changes Jun 5, 2024
View reviewed changes

@ajrbyers ajrbyers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks Joe, these changes look good. We should also consider how this will effect importers in the import plugin. Perhaps open an issue over there to check?

@ajrbyers ajrbyers requested a review from mauromsl June 5, 2024 08:55
ajrbyers and others added 16 commits August 19, 2024 16:00
@ajrbyers @joemull
Switches news item title to a bleachfield #4097
7f248d1
@joemull
Use custom class for bleach charfield #4097
4f50d37
@joemull
Support italics in About homepage element #4097
905ee6d
@joemull
Support italics in Journals and HTML element #4097
bb6bf98
@joemull
Use TinyMCE on HTML block #4097
27a7f61
@joemull
Bleach charfields used in carousel title or subtitle #4097
216a3d1 
I also took the chance to get rid of null=True on these charfields.
@joemull
Display italics in carousel text #4097
27112be
@joemull
Avoid using journal name in license info text #4097
57b11f7
@joemull
Change the rest of the copyright info texts #4097
47436bf
@joemull
Migrate null values to empty strings #4097
9038c5c
@joemull
Fix return statement indentation #4097
ef6c180
@joemull
Fixes to migration dependences and options
5535ab4
@joemull
Use bleached strings for appropriate account fields #4097
bcc9073 
Some FrozenAuthor fields are now bleached and blanks are stored as strings
for simplicity and consistency. Because FrozenAuthor draws on the
corresponding Account fields, we should give them the same treatment.
@joemull
Remove extraneous template call of news item str #4097
916c5e8
@joemull
Merge migrations after rebase #4097
ee9d5dd
@joemull
Avoid mark_safe in posted_by method #4097
85c3a29
@joemull joemull force-pushed the 4097-additional_html_display branch from 9642c9e to 85c3a29 Compare August 19, 2024 15:21
@joemull joemull requested a review from mauromsl August 19, 2024 15:23
@joemull joemull assigned mauromsl and unassigned joemull Aug 19, 2024
@mauromsl mauromsl assigned ajrbyers and unassigned mauromsl Aug 22, 2024
ajrbyers
ajrbyers approved these changes Aug 29, 2024
View reviewed changes

@ajrbyers ajrbyers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🎖️

@ajrbyers ajrbyers merged commit 05df79d into master Aug 29, 2024
@ajrbyers ajrbyers deleted the 4097-additional_html_display branch August 29, 2024 11:56
@joemull joemull mentioned this pull request Oct 16, 2024
Merged
mauromsl added a commit that referenced this pull request Oct 31, 2024
@mauromsl
#4467: Fixed bad render of news in #4153
78212ef
mauromsl added a commit that referenced this pull request Nov 5, 2024
@mauromsl
#4467: Fixed bad render of news in #4153
34a508b
@joemull joemull mentioned this pull request Apr 23, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ajrbyers ajrbyers ajrbyers approved these changes
@mauromsl mauromsl mauromsl approved these changes

Assignees

@ajrbyers ajrbyers

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Italicisation for text fields in Janeway that do not currently allow this, e.g. news item titles

3 participants

@joemull @ajrbyers @mauromsl