-
Notifications
You must be signed in to change notification settings - Fork 9.4k
FIx text and images are displayed cropped if folder has long name && Filters isn`t applied on the "Category" #29633
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
Hi @Nazar65. Thank you for your contribution
❗ Automated tests can be triggered manually with an appropriate comment:
You can find more information about the builds here ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review. For more details, please, review the Magento Contributor Guide documentation. 🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket. 🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
@magento run all tests |
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.
Thanks for the PR @Nazar65 ! Can you please cover the fixed cases with the MFTF tests:
- Creating a folder with more than 50 characters name
- Applying a filter after following the link with url-filter-applier
@@ -84,8 +84,9 @@ private function getDirectories(): array | |||
} | |||
|
|||
$pathArray = explode('/', $path); | |||
$displayName = strlen(end($pathArray)) > 50 ? substr(end($pathArray),0,50)."..." : end($pathArray); |
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.
Please create a constant for 50
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.
ok
@magento run all tests |
@magento run all tests |
@magento run all tests |
$directories[] = [ | ||
'data' => count($pathArray) > 0 ? end($pathArray) : $path, | ||
'data' => count($pathArray) > 0 ? $displayName : $path, |
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.
Not sure it's a right way to trim name on the backend, it would be better adapt frontend application to handle long names. Let's imagine that someone will decide to extend frontend implementation and provide new design (where long names will look good) and with such approach, we are limiting 3d party developers to use trimmed names only.
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.
agree but there is some limitation in a frontend so i decided it can be more easier to fix this in a back-end part, but i will try one more time fix this behavior on a frontend.
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.
@sidolov i'v changed implementation, instead of cutting folders as it was done in old media gallery and there is also this issue here ->
so when we have a long folders and the block is too small for it i had added ability to scroll, as we can have alot of folders created one in one and soon or later we also have same issue, so i think only adding ability to vertical scrolling when there is a lot folder will be correct
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.
@Nazar65 sounds good to me! Look like it's the only solution in such case.
one by one folders revert cutting folders from backend
@magento run all tests |
@magento run all tests |
@magento run all tests |
✔️ QA Passed "We couldn't find any records." text displayed without cropping if there is no image. If there is an image, it displays without cropping. Manual testing scenario |
@sidolov there was some style issue, so i've added few new commits, need to review it again |
@magento run all tests |
Hi @sidolov, thank you for the review.
|
filters on product grid
✔️ QA Passed |
@magento run all tests |
…long name && Filters isn`t applied on the "Category" #29633
Hi @Nazar65, thank you for your contribution! |
Description (*)
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTTEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEESSSSSSSSSSSSSSSSSSSSSSSTTTTTTTTTTTTTT
)"We couldn't find any records." text should be displayed without cropping it there is no any image. If there is any image, it should also be displayed without cropping.
?filters[asset_id]=[1]
part of request from address line -> PressEnter
keyFilter should be applied
Questions or comments
Contribution checklist (*)