-
Notifications
You must be signed in to change notification settings - Fork 309
Remove unused hideOnMobile column definition property. #10906
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
Remove unused hideOnMobile column definition property. #10906
Conversation
Size Change: -554 B (-0.03%) Total Size: 2.13 MB ℹ️ View Unchanged
|
Build files for 47e77d7 have been deleted. |
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 @ankitrox, this is looking good. Two points:
- Please remove the unused SCSS style as per my comment.
- Please can you update the QAB to make it clearer where the focus of the smoke testing should lie. We don't need to spend time smoke testing the entire plugin, only those parts which display the
ReportTable
component.
Thank you @techanvil for reviewing the PR. I have removed the unsed style as suggested and also updated the QAB as per instructions and to focus only on the area which is being changed in this PR. |
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 @ankitrox, that's great. LGTM!
✅
Note that I've confirmed the failing E2E and VRT tests are unrelated to this PR.
Summary
Addresses issue:
hideOnMobile
column definition property fromReportTable
component #10783Relevant technical choices
PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist