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

Theming newtheme #111

Merged
merged 13 commits into from
Jul 18, 2023
Merged

Theming newtheme #111

merged 13 commits into from
Jul 18, 2023

Conversation

ckarpinski
Copy link
Collaborator

Story

I am working on a custom theme to use for our shared search tenant. I have customized the masthead section that shows underneath where the app name, language and login parts are. (see screenshots below) I have been looking at BL to figure out how to do this and for the thing I am having trouble with, i specifically used this file :
https://github.com/scientist-softserv/britishlibrary/blob/0c9299d6c16c2e60a1b91a71eba7a421081e55a1/app/views/hyrax/homepage/_banner_section.html.erb#L62

and put my version of that code into this page:
https://github.com/scientist-softserv/britishlibrary/blob/0c9299d6c16c2e60a1b91a71eba7a421081e55a1/app/views/themes/bl_shared_home/layouts/homepage.html.erb

The file I made is - app/views/themes/shared_repository/layouts/homepage.html.erb

Expected Behavior After Changes

It all seems to be working correctly except for the search results page. In my file on line 34 it is saying
<% if controller_name == 'catalog' %>
that seems to have no affect, while the code above that for homepage, pages and contact_form
<% if controller_name == 'homepage' || controller_name == 'pages' || controller_name == 'contact_form' %>
all work output the code that is there for those pages.

Additionally the footer.html.erb file in app/views/themes/shared_repository/shared/_footer.html.erb is working everywhere but on the search results page. This seems likely related.

Screenshots / Video

masthead section before
Screenshot 2023-07-10 at 1 31 59 PM

mast head section with the new theme
Screenshot 2023-07-10 at 1 33 49 PM

masthead section with new theme not working on the search results page
Screenshot 2023-07-10 at 1 41 19 PM

Footer with the theme working:
Screenshot 2023-07-10 at 1 51 48 PM

footer not working on the search results page
Screenshot 2023-07-10 at 1 51 11 PM

Notes

I see in app/controllers/hyrax there are files for pages_controller.rb, homepage_controller.rb and contact_form_controller.rb but not catalog_controller.rb - that one is one level up. I dont know if this has anything to do with it. I am thinking this because BL's references to <% if controller_name == 'catalog' %> are not inside a theme and maybe that affects how that works or doesnt work.


<% if controller_name == 'homepage' ||
controller_name == 'pages' ||
controller_name == 'contact_form'
Copy link
Collaborator

Choose a reason for hiding this comment

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

did you already try this?

Suggested change
controller_name == 'contact_form'
controller_name == 'contact_form' ||
controller_name == 'catalog'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason they are separate is that there is a different layout for the top of the catalog page. I was not yet sure if it was similar enough to make that change like i did for the home page on line 24 so i kept it separate. BUT, I just now tried removing the section and adding catalog with the others and it has no affect.

Copy link
Collaborator

Choose a reason for hiding this comment

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

inside this view, try adding a debugging statement like
<%= raise 'debug' %>
it will show an error page, with a console on it. in the console you can write controller_name to check what the controller is called on that page.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@summer-cook I added it but only get the error on the pages that were working - it has no affect on the catalog page. so that page is not being used at all when the catalog gets loaded.

</div>
<%end%>

<% if controller_name == 'catalog' %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

if the suggestion above works, remove this whole section until line 54

@@ -6,6 +6,7 @@
<%= content_for(:head) %>
</head>
<% content_for(:extra_body_classes, 'public-facing') unless params[:controller].match(/^proprietor/) %>
<% content_for(:extra_body_classes, ' search-only') if current_account && current_account.search_only %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

check in the inspector in the rendered html that the search-only body class is being correctly applied on the search results page, because it might not be. I noticed you have a space in the string so it reads ' search-only' which may be incorrect.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So without that space it makes public-facing and search-only into public-facingsearch-only and all the css stops working.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I put the debugger bit in this file and reloaded the search results page and then put controller_name and it said "catalog" so I think that means I have the right name for it.

Copy link
Collaborator

@summer-cook summer-cook left a comment

Choose a reason for hiding this comment

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

British libraries uses a different layout for inside the search pages (catalog) than for their homepage and elsewhere in the app.

Also, in regards to your comment about catalog_controller.rb being one level up, that wouldn't matter

@ckarpinski
Copy link
Collaborator Author

ckarpinski commented Jul 10, 2023

@summer-cook the catalog page is not loading the same masthead file as the other pages. I have a custom _masthead.html.erb file that is being used everywhere but catalog. So the catalog gets the body classes from the file but when it gets to <%= render '/masthead' %> it uses a different file than everywhere else (same with <%= render 'shared/footer' %>)

@ckarpinski ckarpinski marked this pull request as ready for review July 17, 2023 16:42
Copy link
Collaborator

@summer-cook summer-cook left a comment

Choose a reason for hiding this comment

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

I know i already approved this one, but make sure rubocop is passing before merging. let me know once it is passing and ill re-approve

@summer-cook summer-cook merged commit 8602eba into main Jul 18, 2023
4 of 7 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