Skip to content

Conversation

@markconroy
Copy link
Member

Closes #123

What does this change?

  • Adds a new field called "List Layout" so editors can choose on a per-guide basis whether they want horizonal/vertical layout.
  • Defaults to "Vertical (Single Column)"

Screenshot 2024-11-28 at 14 53 02

How to test

Install Guides module
Create a guide overview and some guide pages
In Guide Overview, choose "Horizontal" or "Vertical" for the "List Layout"
Check that the Guide Nav responds accordingly NOTE it will need this PR from LocalGov base which has the required CSS.


Thanks to Big Blue Door for sponsoring my time to work on this.

Copy link
Member

@msayoung msayoung left a comment

Choose a reason for hiding this comment

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

Causes errors on pre-existing guides.

InvalidArgumentException: Field localgov_guides_list_layout is unknown.

@stephen-cox
Copy link
Member

Causes errors on pre-existing guides.

InvalidArgumentException: Field localgov_guides_list_layout is unknown.

You're going to need to either check the field exists before using it or add an update hook to install this into existing sites.

@markconroy
Copy link
Member Author

@msayoung @stephen-cox this PR is updated now to check for the field before acting on it.

Do we want to add this field to existing installs as well as new installs?

@markconroy markconroy requested a review from msayoung December 13, 2024 16:17
@stephen-cox
Copy link
Member

I will leave the decision on whether to add this to existing installs to someone else, but I not it's not straightforward to add is manually so might be good to push it out if it's useful.

Looks fine to me now, but before we can merge this we'll want all the tests passing.

@msayoung msayoung self-requested a review December 17, 2024 11:52
@msayoung
Copy link
Member

Also just tested and working fine. Good question about existing sites, I agree that probably yes ...?

@stephen-cox
Copy link
Member

Discussing at Merge Tuesday - would a site wide setting for this be better?

@finnlewis
Copy link
Member

@markconroy do you think a site wide setting would be better here?

@ekes ekes marked this pull request as draft January 14, 2025 12:10
@markconroy
Copy link
Member Author

Closing this, as we now have a sitewide setting instead.

@markconroy markconroy closed this Mar 4, 2025
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.

Consider vertical navigation for guides

5 participants