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

Hotfix/8.8.3 - Enhanced text-overlay design on mobile to match hero-half #695

Closed
wants to merge 3 commits into from

Conversation

breakdancingcat
Copy link
Member

I noticed Text-overlay mobile design was lacking, mirrored to the half-hero mobile appearhance (without a bold title)

Half mobile Text overlay mobile
Screenshot 2024-02-15 at 4 09 13 PM After Screenshot 2024-02-15 at 4 09 33 PM
x Before Screenshot 2024-02-15 at 4 12 42 PM

@coveralls
Copy link

coveralls commented Feb 15, 2024

Coverage Status

coverage: 100.0%. remained the same
when pulling e02a4a2 on hotfix/8.8.3
into 4dc1052 on master.

@breakdancingcat breakdancingcat deleted the hotfix/8.8.3 branch February 15, 2024 22:19
@yield('hero-buttons')
</div>
<{{ !empty($hero['link']) ? '/a' : '/div' }}>
Copy link
Member

Choose a reason for hiding this comment

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

Feel like the tradeoff of the performance of this isn't worth the inconsistency with splitting this tag up.

Could we stick to full tags here?
{{ !empty($hero['link']) ? '</a>' : '</div>' }}

@if(!empty($hero['description']))
{!! $hero['description'] !!}
@if(!empty($hero['link']))
{!! preg_replace(array('"<a href(.*?)>"', '"</a>"'), array('',''), $hero['description']) !!}
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we're starting to do more than display in the templates. Let's take a look at long-term moving some of this content formatting into the controllers/repositories before it gets to Blade to display.

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.

👍

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.

5 participants