Skip to content

Conversation

StephDriver
Copy link
Contributor

closes #4694

@StephDriver
Copy link
Contributor Author

StephDriver commented Sep 23, 2025

This includes new settings as both press and journal level. Content is just placeholder - there is another issue to write it, but I am anticipating Theme specific content (by adding theme a11y templates) with some repeated parts (Janeway, Press, Journal) which I've added as common templates.

Behaviour

  • When an a11y info text field is left empty, the template doesn't display that section (rather than display a blank section).
  • At press level there are two settings, one for the accessibility information, the other a boolean for whether journals can have their own information as well.
  • At Journal level, if and only if the boolean above is true, then there is a setting for accessibility text in the General journal settings.
  • Regardless of whether the a11y text exists at journal level, if in the future the press boolean changes, it will not display (but is not lost).

On Press

Press Manager | Edit Press Details

image

For Journals

Journal Manager | General
Note, this section only displays if the "allow journal accessibility information" setting at press level is enabled.

image

@StephDriver StephDriver requested a review from joemull September 23, 2025 07:42
@StephDriver StephDriver marked this pull request as ready for review September 23, 2025 07:42
@StephDriver
Copy link
Contributor Author

Just realised the clean theme journal footer has the accessibility link and contact link swapped around. Adding another commit to fix this and make it consistent with the order everywhere else.

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.

The pages are great. I have a few suggestions about the data model and logic of the press-dependent setting mechanism.

# Non-press-dependent settings are always included
filtered_settings.append(setting)

settings = filtered_settings
Copy link
Member

Choose a reason for hiding this comment

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

I'm not so sure the settings should be invisible at the journal level. We already often have the problem that people don't understand why they can't see something that other people tell them should be there. This would re-create that problem in a new place unless someone knows that these settings are interrelated at the press and journal level. Staff might not always know how to work this feature, or communicate it to the journal editors.

What if instead of disappearing, the settings were read-only and could not be edited?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did wonder about this for some time while working on the branch. My feeling was that in this specific instance, no or at least not yet. because I think the accessibility bit needs to be rolled out more slowly. Likely first to Press managers, so they can think about their part, and whether they want journal staff to see this or not. And only then to make it visible.

What I want to avoid is journal staff seeing the setting, and it's also the first time they've seen a setting that is not active. Particularly as by making this an 'a11y' group setting, it appears by default at the start of the all settings list.

Longer term, yes, I do think they should be visible but inactive and signposted as 'needs X setting on press'. But I don't want to introduce both inactive settings and accessibility page at the same time.

},
"editable_by": [
"journal-manager"
]
Copy link
Member

Choose a reason for hiding this comment

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

Just to record what we discussed off GitHub, the is_press_dependent field will need to be extended to all items in the JSON file, for consistency and maintainability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at doing this - but the file is not consistent as is - the setting attributes aren't always in the same order - so making it consistent feels like a stand alone task.

@joemull joemull assigned StephDriver and unassigned joemull Sep 23, 2025
@StephDriver StephDriver requested a review from joemull September 26, 2025 09:34
@StephDriver StephDriver assigned joemull and unassigned StephDriver Sep 26, 2025
@StephDriver StephDriver force-pushed the b-4694-setup-a11y-info branch from 02ec4a6 to d6f408c Compare September 26, 2025 09:55
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.

Front of House Accessibility Information Page

2 participants