search

CDRH / orchid

Rails Engine for site integration with CDRH API
MIT License
3 stars 0 forks source link

Consider site_section vs @section #141

Closed jduss4 closed 4 years ago

jduss4 commented 4 years ago

We have previously used site_section to indicate that this is a portion of a site, like "memorabilia" for Trans-Mississippi Expo, or "About" pages for a project, etc, but now with the @section work this term may not be the best fit anymore.

Should we consider renaming it? Or just making the distinction very clear?

Additionally, I think it would be appropriate to add @section to that same location as a class, and also if there is some way to automatically attempt to grab a SCSS file named the same as the section.....I feel like that could go a long way towards adding a huge component to styling future versions of the api_frontend. Not critical at this moment, but thinking about adding that class is how I became aware of the site_section issue.

https://github.com/CDRH/orchid/blob/dev/app/views/layouts/application.html.erb#L2

techgique commented 4 years ago

Updating site_section logic to use @section seems like a solid move to consolidate things. Like you mentioned, we'll have to do a good job of not conflating general @section use with Orchid's internal @section use. :-)

Checking for section Sass and JavaScript files seems like a good idea. I think that's something that we'd want to handle within an initializer so the app isn't checking the file system on every request within a section's URI scope. Add another key to the SECTIONS hash which is a boolean of whether or not the section has its own files in app/assets/stylesheets/(sections/) and app/assets/javascripts/(sections/) :+1:

jduss4 commented 4 years ago

Sorry, just to be clear, I don't know if I was advocating for site_section to use @section necessarily.....but wondering if we should rename site_section to something else to be less confusing? I think we would break quite a lot of stuff throughout our existing sites if we were to combine the functionality or only use @section brb more coming later

jduss4 commented 4 years ago

Now I am back :) But yeah, I like the idea of doing some kind of section hash for the various assets, the way that we already are sort of doing for loading the configuration and api settings and such!

But in terms of keeping the current functionality of the site_section I am not sure what way to roll.....because things like writings as a section to tie into the navigation feels different than writings/letters, which is a @section, for example. I dunno, maybe we'd need to look at some examples from various sites.

techgique commented 4 years ago

Ah okay. Your example of all the writings/ pages (a step above the Orchid "section" writings/letters/) relying on the same class in <html> for CSS and nav may be the perfect exception to just using @section in place of site_section. We wouldn't want the Orchid "section" to override in that case and set the class letters on <html> right?

We could handle that pretty easily in Sass by just using both the .writings and .letters selectors for the Sass parent class around the nested stylings. But the nav behavior may not be as easily addressed.

I'd like to try to keep these things simpler by having them all work together with @section but it may not be possible or easy. Otherwise I worry we'll be adding yet more jargon to how Orchid works. :sweat_smile:

But if they do need to be independent from each other, I suppose we could use another term like "namespace" or "(page)grouping" or "(page)set"? It's just tricky to define all these terms and keep them all straight :dizzy_face: :smile:

jduss4 commented 4 years ago

Agreed about keeping the terms straight. I like the idea of mostly relying on @section but having some sort of variable still available if you need a different selector or more specificity. I think we may need to discuss with @karindalziel about this one, since she's the one typically using that @site_section for things.

jduss4 commented 4 years ago

This was resolved in #188 , although I notice that there's one template still referencing it which I missed, so I will close this issue and make a new one for that.