-
Notifications
You must be signed in to change notification settings - Fork 4
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
Added the logic for Japannese search pages for description limiting t… #540
Conversation
Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
|
|
@@ -132,7 +132,9 @@ async function searchPages(placeholders, term, page) { | |||
header.appendChild(link); | |||
res.appendChild(header); | |||
const para = document.createElement('p'); | |||
setResultValue(para, line.description, term); | |||
if (getLanguage() === 'jp') { |
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.
LGTM if @ehrro is fine with this logic.
Hello it looks good to me. Thanks. |
setResultValue(para, line.description, term); | ||
if (getLanguage() === 'jp') { | ||
setResultValue(para, line.description.split('。')[0], term); | ||
} else setResultValue(para, line.description, term); |
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.
Why just jp
? Can't the same problem happen in any language with long description?
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.
Only jp
has 。
so there should be another if
for the other languages that use .
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.
Also maybe add ja
as well to be safe, I don't think we have them mixed anywhere but better safe than sorry
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 mean what if the line.description.
is long enough to require trimming.
…o 1 line
Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after), along with a short summary of changes:
#537
Fixes #537
Changelog:
Please enter each change as a new bullet point
Test URLs:
Library
New Blocks introduced in this PR
If yes, please provide details below
Block Name : For e.g. cards
Documented in sidekick library
New variations introduced in this PR
Variation Name : For e.g. cards (grid)
Documented in sidekick library
New mixins introduced in this PR
Mixin Name : For e.g. compact, white
Documented in sidekick library