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

FIX | Menu Top - Heading Title Single/Double | Wrapper Height Consistency #778

Closed
wants to merge 3 commits into from

Conversation

KyleApsey
Copy link

Reason for change

Why this change is taking place?

Padding updates made for the sur-title from #777 added height to the menu-top wrapper.
Basecamp reference

Updates

  • Added conditional wrapper padding adjustment based on presence of sur-title for menu-top

Relevant links

Reminders

  • Update package version number
  • Frontend accessibility check
  • Styleguide example in place
  • New functionality covered by tests
  • Screenshots of multiple screen sizes

Demo

Before After
conditional-padding-before conditional-padding-after

@KyleApsey KyleApsey requested a review from a team as a code owner February 5, 2025 14:48
@coveralls
Copy link

coveralls commented Feb 5, 2025

Coverage Status

coverage: 100.0%. remained the same
when pulling e615128 on feature/menu-top-wrapper-padding
into f027ad4 on develop.

@KyleApsey KyleApsey marked this pull request as draft February 5, 2025 14:54
@KyleApsey KyleApsey marked this pull request as ready for review February 5, 2025 15:00
Copy link
Member

@nickdenardis nickdenardis left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@KyleApsey KyleApsey closed this Feb 5, 2025
@KyleApsey KyleApsey deleted the feature/menu-top-wrapper-padding branch February 5, 2025 18:45
@chrispelzer
Copy link
Member

👍🏼 I like the use of the @php to keep the template layout cleaner.

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.

4 participants