Tweak SnippetPreview #21469
Open
Tweak SnippetPreview #21469
+102
−88
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reasoning: mobile users drag instead of scroll
Needed to be able to influence the fixed width background color * using the given className extended with `__wrapper` * add fallback for the className: fallback to the ID * add className (same as the ID) to the FixedWidthContainer used in the SnippetPreview
For ease of development
In the current font size: by 3px
igorschoester
added
changelog: enhancement
Making it possible to change the padding via CSS by checking the offsetWidth and comparing it against the scrollWidth. This assumes the max-width and overflow remain in effect!
| @@ -111,12 +111,13 @@ export default class FixedWidthContainer extends Component { | |||
| render() { | |||
| const { width, padding, children, className, id } = this.props; | |||
|
|
|||
| const klass = className || id; | |||
There was a problem hiding this comment.
Suggested change
| const klass = className || id; | |
| const classNameOrId = className || id; |
| const innerWidth = width - 2 * padding; | ||
|
|
||
| return <React.Fragment> | ||
| return <div className={ `${ klass }__wrapper` }> |
There was a problem hiding this comment.
Suggested change
| return <div className={ `${ klass }__wrapper` }> | |
| return <div className={ `${ classNameOrId }__wrapper` }> |
| <FixedWidth | ||
| id={ id } | ||
| className={ className } | ||
| className={ klass } |
There was a problem hiding this comment.
Suggested change
| className={ klass } | |
| className={ classNameOrId } |
| this.setState( { | ||
| showScrollHint: width < this.props.width, | ||
| showScrollHint: this._container?.offsetWidth !== this._container?.scrollWidth, | ||
| isMobileUserAgent: window?.navigator?.userAgent?.includes( "Mobi" ), |
There was a problem hiding this comment.
What about a case insensitive check?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
Summary
This PR can be summarized in the following changelog entry:
widthprop is larger than the currentwidth.yarn watchcommand for ease of development.Relevant technical choices:
Mobi, to determine if on a mobile device. Not sure what else to use whenuserAgentDatais not fully supported (see https://caniuse.com/?search=navigator.userAgentData). I sawmaxTouchPointsbeing suggested (see https://developer.mozilla.org/en-US/docs/Web/API/Navigator/maxTouchPoints), I'm not sure.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
New copy
Drag to view the full preview.Scroll to see the preview content.Regression tests
DEV only: yarn watch
yarn workspace @yoast/search-metadata-previews watch, or runyarn watchin thepackages/search-metadata-previewsfolderpackages/search-metadata-previewsyarn startin the root), you would see the change on refresh of the pageRelevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
UI changes
Other environments
[shopify-seo], added test instructions for Shopify and attached theShopifylabel to this PR.Documentation
Quality assurance
Innovation
innovationlabel.Fixes https://github.com/Yoast/ux/issues/140