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

block--with-button "include" template updates #69

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alisonjo315
Copy link
Collaborator

@alisonjo315 alisonjo315 commented Jul 17, 2023

These template changes are based on changes made on LASSP/Kavli.

Included changes:

  • Allow class overrides
  • Allow placing the "more link" at the top or the bottom (rather than only at the top)
  • Clean up docblock

To see the changes in action:

also, clean up docblock (copied from LASSP/Kavli)
@alisonjo315 alisonjo315 requested a review from ama39 July 17, 2023 21:38
Copy link
Collaborator

@ama39 ama39 left a comment

Choose a reason for hiding this comment

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

It's possible you may need a <div class="constrain-480"> wrapper in the footer placement as well, but I can't be sure without seeing an example rendered. This 480 constraint is used to keep things aligned at certain awkward device sizes (the transition between phone and tablet).

@alisonjo315
Copy link
Collaborator Author

alisonjo315 commented Aug 24, 2023

@ama39 Oops, I definitely should've included links in the PR description 🤦‍♀️ [adding links now, in addition to here in this comment]

The "changed version" of this block template is in use on the LASSP homepage, for both the News and Events blocks:
https://www.lassp.cornell.edu/

The "Related news" block on People pages uses the (current/un-changed) cwd_base version of the block template in question:
https://www.lassp.cornell.edu/people/tomas-arias

I just made an update on a LASSP multidev, to have that "people related news" block use the "changed version" of the block template, to confirm the changes won't mess anything up on existing uses of this block template:
https://drupaug23-lassp.pantheonsite.io/people/tomas-arias

@alisonjo315 alisonjo315 changed the title block-with-button: allow class overrides block--with-button "include" template updates Aug 24, 2023
@alisonjo315 alisonjo315 requested a review from ama39 August 24, 2023 20:36
@alisonjo315
Copy link
Collaborator Author

@ama39 I think this is good now, I think?

@alisonjo315
Copy link
Collaborator Author

@ama39 Do you think you could look at this PR this week, or unlikely? Thanks!

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