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

Break apart /less/inc/widgets.less into per-widget files #882

Merged
merged 5 commits into from Sep 3, 2015

Conversation

benlk
Copy link
Collaborator

@benlk benlk commented Aug 31, 2015

Changes

  • widgets.less is now many per-widget files
  • footer.less loses some styles
  • Grunt now watches all files in all subdirectories of /less/inc

Why

To clean up widgets.less, for #880

To Do

  • Go over other theme LESS files and search for hiding widget css rules.
  • Visual tests to make sure nothing went missing or broke. (Should be ok, though)
  • How zealous should we be in getting widget css out of other files?
    • Should this break up the Responsive media query files?
    • Should this pull styles out of single.less if the widget is only ever displayed in single.less?

…iles to the Grunt watch task, and clean up a little
@benlk
Copy link
Collaborator Author

benlk commented Sep 1, 2015

List of widget classes and ids, for the search:

.largo-about
.largo-author
    .author-box
.largo-disclaimer
.largo-donate
.largo-explore-related
    #related-posts
    #related-post-nav
    #related-items
.largo-facebook
    .fb-like-box
.largo-featured
.largo-follow
.largo-footer-featured
.widget_sp_image
.largo-INN-RSS
.largo-post-series-links
.largo-prev-next-post-links
    .previous
    .next
.largo-recent-comments
    .recentcomments
    .comment-excerpt
    .comment-meta
.largo-recent-posts
.largo-related-posts
    ul.related
.largo-series-posts
    h4
.largo-sidebar-featured
.largo_staff_widget
    ul.staff-roster
    span.staff-name
.largo-tag-list
    .tags
.largo-taxonomy-list
    select.taxonomy-list-widget
.largo-twitter
    a.twitter-timeline

@aschweigert
Copy link

This seems like a solid start but I'm not 100% sold on the need for the empty files. I'd say we should probably just create separate files for the widgets that actually have those more narrowly scoped styles associated with them and try, as much as possible, to just use the defaults wherever possible and override things only very selectively.

@benlk
Copy link
Collaborator Author

benlk commented Sep 1, 2015

I agree with you on the totally-empty files, but I think we should keep empty files for the comments when there are more-narrowly-scoped styles specific to that widget that are kept in a different file to reduce code duplication.

@aschweigert
Copy link

Hm. Maybe. I think it makes sense to do that sometimes. Like, if there's a reasonably likely chance that a developer working on this would be confused, then thumbs up. If it's just a placeholder, then probably not necessary.

@benlk
Copy link
Collaborator Author

benlk commented Sep 1, 2015

I guess the question is: What would cause confusion?

If someone's looking at per-widget LESS files, they probably wouldn't look for styles concerning the INN Member Stories widget in largo-featured-posts.less. Having the empty file to point to the correct file reduces that confusion. Same with largo-tag-list.less, which points to /less/inc/single.less, because that's where the styles live, to reduce duplication and to make sure that changes to other styles catch the tag list widget.

If someone's looking at /less/inc/widgets.less, then there are comments in that file so people don't go to the empty file. They'll go to the correct file.

If someone's looking at the sourcemaps in a browser, they're going to see the correct file, and won't ever see the empty file, so I don't think the empties will cause confusion.

@benlk
Copy link
Collaborator Author

benlk commented Sep 3, 2015

This now has the empty files removed.

rnagle added a commit that referenced this pull request Sep 3, 2015
Break apart /less/inc/widgets.less into per-widget files
@rnagle rnagle merged commit ca9f1d1 into develop Sep 3, 2015
@rnagle rnagle deleted the 880-split-up-widgets-css branch September 10, 2015 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants