-
Notifications
You must be signed in to change notification settings - Fork 28
Improve narrow YouTube Click to Load placeholders #422
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
Conversation
1395c62
to
23bf8f9
Compare
23bf8f9
to
cd72c13
Compare
def7e31
to
96c1d3a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're handling well these changes. I wonder if it would be best to create a new text in the config instead of moving the "learn more" around. But there wouldn't be much gain either way, and like you said, we will likely refactor this code soon, so it should be fine to leave it like this to unblock you. I left some other general comments, ping me if you have any questions :)
Yea, you're right there. Moving the string around like this isn't correct really, and with the new preview sub-heading string the link is included. I figured this is pragmatic for now though, in particular since we're doing the same thing when adding this particular link originally - we append it to a div with the text content of |
96c1d3a
to
82a4010
Compare
When the YouTube Click to Load placeholders are very narrow, the information text takes up too much space and some of the other elements are lost. Let's tweak narrow placeholders, to hide that information text and move the "Learn more" link. This isn't the best approach (using @container queries[1] might be better), but this code is going to be refactored shortly anyway. 1 - https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Container_Queries
82a4010
to
c83b993
Compare
When the YouTube Click to Load placeholders are very narrow, the
information text takes up too much space and some of the other
elements are lost. Let's tweak narrow placeholders, to hide that
information text and move the "Learn more" link. This isn't the best
approach (using @container queries[1] might be better), but this code
is going to be refactored shortly anyway.
1 - https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Container_Queries