Resize bug with dialog.views.js

I'll take a look.

Thanks for the patch. I still need to test this but I've reviewed the code:

1) I reviewed the removed JavaScript and it is only for resizing the dialog

2) I checked all the code to see if js/dialog.views.js was added anywhere else and it was only in views_ui.libraries.yml (and it is removed from that libraries file in the patch)

3) I looked at the CSS changes and they seem ok

4) I checked other CSS for places where it had similar CSS to Claro and found:

.views-ui-dialog.views-ui-dialog-scroll .ui-dialog-titlebar {
  border: none;
}

in:

• core/modules/views_ui/css/views_ui.admin.theme.css
• core/themes/stable/css/views_ui/views_ui.admin.theme.css
• core/themes/stable9/css/views_ui/views_ui.admin.theme.css

So I'm wondering if the same issue is present for these themes and the same CSS change is needed for those files.

@Kristen Pol

I have removed css from rest of file.

Updated file

Thanks for the update @sanjayk. Fyi, it's good to provide an interdiff file when making changes so it's easier to see what has changed.

1) In reviewing the changes, I see that:

.views-ui-dialog.views-ui-dialog-scroll .ui-dialog-titlebar {
  border: none;
}

was removed from:

• core/modules/views_ui/css/views_ui.admin.theme.css
• core/themes/stable/css/views_ui/views_ui.admin.theme.css
• core/themes/stable9/css/views_ui/views_ui.admin.theme.css

and no other changes were made which is as expected.

2) The patch applies cleanly:

[mac:kristen:drupal-9.0.x-dev]$ patch -p1 < 3118010-3_0.patch
patching file core/modules/views_ui/js/dialog.views.es6.js
patching file core/modules/views_ui/js/dialog.views.js
patching file core/modules/views_ui/css/views_ui.admin.theme.css
patching file core/modules/views_ui/views_ui.libraries.yml
patching file core/themes/claro/css/theme/views_ui.admin.theme.css
patching file core/themes/claro/css/theme/views_ui.admin.theme.pcss.css
patching file core/themes/stable/css/views_ui/views_ui.admin.theme.css
patching file core/themes/stable9/css/views_ui/views_ui.admin.theme.css

3) This will need to be manually test for these themes for sure:

• claro
• stable
• stable9

but not sure what else because this CSS:

core/modules/views_ui/css/views_ui.admin.theme.css

is in the module.

Thanks @bnjmnm. That is good to understand. So it needs to go back into stable and stable9. Is there an issue removing it from:

core/modules/views_ui/css/views_ui.admin.theme.css

? If so, then your patch from #2 is the keeper. Otherwise, we need a new patch with just that file updated.

Assigning to myself to review.

I'm getting pulled away for a bit so unassigning for now in case someone else can review.

Reviewed #11 and #13 and these changes seem fine based on the discussion. Test pass now so this just needs manual testing. I can't get to that tonight unfortunately.

I tested on #13 on Chrome and do not see the dark line when I resize the window up and down.

I should have noted that I was able to reproduce the issue without the patch.

Adding tag as I worked on this yesterday for the Bug Smash Initiative.

Note that I didn't marked RTBC because it might be good for someone with more experienced JS/CSS chops to scan the patch.

We have some Javascript tests that use the dialog, but we don't have anything that tests the resizing (as far as I can find). While I don't advocate testing this pixel perfect, adding a test that the resize still happens would not be a bad thing I think.

I'm all for removing code. So patch is good on that front +1

On the testing front today we have that: https://git.drupalcode.org/project/drupal/-/blob/9.0.x/core/tests/Drupal... so not a lot.

If we want to add a test I don't think the test needs to be view ui specific might be enough to check that the size of the dialog changed before and after browser resize in that method.

@nod_ Thanks for the input!

might be enough to check that the size of the dialog changed before and after browser resize in that method.

I think I'm missing the point. From what you wrote, it looks like you are suggesting that we might add a test to see if the dialog size changes. But, this bug isn't that the dialog doesn't resize properly, just that there is a slight visual anomaly when it does resize. Shouldn't the test added be for the bug in question?

Tested with above mentioned points and patch.
Its appearing good on chrome browser. RTBC + 1

@tanubansal Thank you for your help but it was already manually tested in #17. Please don't duplicate work if possible. Thanks.

It was to complement #21. I think the idea was to make sure the resize worked because we broke views UI quite frequently in the past.

@nod_ Thanks for the update. Tagging for tests.

Back to "Needs work" for tests.

@Kristen Pol I am trying to work on the test cases. I just try to apply the path(#13) but it failed & I found StableDecoupledTest.php not available.
Can you pls guide me to resolve this issue.

Work only for the test cases & updating it's status to need review.

Re-roll for 9.2.x

Tested on both claro and seven and was not able to reproduce. Going to close out but if still an issue please reopen.