Refactor Claro's dialog stylesheet

Gauravvv → made their first commit to this issue’s fork.

More nesting can be done. Left comments in MR.

Screenshots will be needed also.

/**
 * Off-canvas styles.
 */
.ui-dialog.ui-dialog-off-canvas .ui-widget-content.ui-dialog-content {
  background: none;
}

@media all and (max-width: 48em) { /* 768px */
  .ui-dialog:not(.ui-dialog-off-canvas) {
    min-width: 92%;
    max-width: 92%;
  }
}

Think we can fold these in to the parent.

attaching some screenshots to show nothing broke.

There seems to be a regression on rtl the dialog is stuck on the right of the screen, it's not centered anymore.

Tested the MR#3546

Patch applied Clearly
Set the direction RTL and tested UI and popup is aligned perfectly center for reference attaching the screenshot

RTBC + 1

Additional changes by Gauravvv looks good.

Gitlab sees a merge conflict beyond the usual rebase, probably just a little out of sync with HEAD. I spotted two additional nesting opportunities as well, check the MR, so at least the additional work isn't just due to the post RTBC wait.

Addressed feedbacks and rebased the MR

working on it

“I noticed that comment #22 did not incorporate the feedback provided in comment #21 regarding the nesting opportunities. I have worked on the suggestions provided in and made the necessary changes. Tested the ui nothing broken. Please review the MR.

FYI this was tagged for drupalSouth to be looked at there. So if you’re there great

Seems to have a CI build.

I doubt an issue marked as Task will be added to the 10.1.x release since a beta has been released.

I doubt an issue marked as Task will be added to the 10.1.x release since a beta has been released.

Good looking out. Fortunately the Claro stylesheet refactoring issues are incredibly nondisruptive and I'd have no issue committing them to a beta. It's also fine to continue work on the 10.1.x branch in general. We can bump the version up when necessary.

Ah yes, didn't think about that 🙂 thanks @bnjmnm

Cleaning up tags.

Believe all threads have been addressed so don't mind marking.

The MR needs to be rebased on 11.x

working on this

Hi,
In drupal-3303546, 11.x branch is not available to rebase. 11.0.x is present there but it is empty. So, unless we have 11.x branch with all the code, rebasing can't be done.
Thanks

Hi @lauriii I have rebased MR to 11.x.
Please check now.
Thanks!

Not sure if the rebase was done correctly.

"composer.lock" does not contain valid JSON
Parse error on line 686:
...-vendor-hardening",<<<<<<< HEAD
---------------------^
Expected: 'STRING' - It appears you have an extra trailing comma

The failing spellcheck might be a false positive.

Ah, I see the target branch of the MR is not set to 11.x

MRs need to be cleaned up. Both seem to have failures., but should only be 1

Fixed linting errors and updated logical properties. please review

Have multiple MRs, patches up which is it? Can those be cleaned up please.

Gauravvvv → changed the visibility of the branch 3303546-refactor-claros-dialog to hidden.

Gauravvvv → changed the visibility of the branch 11.x to hidden.

Gauravvvv → changed the visibility of the branch 10.1.x to hidden.

Gauravvvv → changed the visibility of the branch 3303546-refactor-claro-dialog-D11 to hidden.

Gauravvvv → changed the visibility of the branch 3303546-11.0.x to hidden.

I have hidden all redundant Merge Requests and patches. Merge Request !5874 is now ready for review.

Thanks, refactoring seems good in the MR that's up.

@sourabhjain Never edit .css files directly. It's clearly mentioned on top of the file. Also, this has been brought up before. Don't just blindly apply code suggestions or merge request remarks.

Thanks for fixing that @BramBriesen.

Seems a forced-color image has been removed, can someone check in forced color mode? thx.

Mithun S → made their first commit to this issue’s fork.

Validated the missing css and added it to the dialog.pcss.css file and compiled and pushed a commit to the branch.
Moving the issue to Needs review.

MR appears to have failures.

I am requesting a REVIEW for a new MR. Thanks )

finnsky → changed the visibility of the branch 3303546-claro-dialog-refactor-d11 to hidden.

Thank you!

CSS changes looks good to me.
Also tested basically and didn't found visual regressions.

+1 RTBC. Would be cool if anyone can make before/after screenshots.

I used view dialog for example and things appear to be unchanged

Before

After

I didn't find unanswered questions in the comments or the MR. A reminder that this issue summary be improved by having the before/after screenshots available from the issue summary and the remaining tasks up to date.

nod_ → closed merge request !9685

Committed and pushed 903f73fb30 to 11.x and fbad2d37f6 to 11.1.x and 431532d69a to 10.5.x and 8c1d5599ef to 10.4.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.