Fix a modal window

Can you please provide steps to reproduce.

Hi,

Just use any link or view and replace with a href like :

<a class="use-ajax" 
    data-dialog-options="{&quot;width&quot;:400}" 
    data-dialog-type="modal" 
    href="/node/1">
    First node displayed in modal dialog.
</a>

Hi,

Just use any link or view and replace with a href like :

<a class="use-ajax" 
    data-dialog-options="{&quot;width&quot;:400}" 
    data-dialog-type="modal" 
    href="/node/1">
    First node displayed in modal dialog.
</a>

More details here: Drupal modal →

Hi

I have setup this in drupal 10 and I am not getting this error. It is working fine. Here is the screenshot for refrence.

Well, from your picture I would say you are using different theme instead. But still normal content works fine here, just admin things like author, publish etc does not as statet in my picture.

I have also installed the same theme but the color of the theme was diffrent. Here take a look at this screenshot.

You are probably view node not edit one. Because in my case i can update field.

I have two questions,
First can you make a video of the issue from the starting and second can you tell me what is you opening in the modal.

Hi @coaston, what is the status of the issue.

Hi i have unistalled rigel theme already because of missing regions. I believe my description was clear.

You need to use href="/node/1/edit" instead and logged as admin. Because admin can see the date of last saved or author etc..

Hi @yas, can you please look into this issue. Because this is not reproducing in my local setup.
Please test this issue in your machine.

@shubham_jain, Thank you for helping on this issue. I was able to reproduce this. The snippet I used is something like this:

    <a class="use-ajax" href="/node/2/edit" data-dialog-options="{&quot;width&quot;:800}" data-dialog-type="modal">First node displayed in modal dialog.</a>

Note that @coaston is referring to using the modal to access an edit page.

See this screenshot of it using Rigel

Then I tried it using Bootstrap5 base theme:

Do you want to help by submitting a patch to fix the issue?

Thanks for being active in this project!

Hi @baldwinlouie, sure will raise PR for this.

@shubham_jain, Thank you so you much for the patch. This is a great start. I have the following feedback.

1. When the modal pops up, the background is white. Some of the text is hard to see because it is also white. Is it possible to keep the modal background one of the theme colors?

2. I like that you fixed the layout of the vertical tabs. However, is it possible to keep them as vertical tabs like in
?

These are the changes I'm talking about.

Thank you so much for your help!

Hi @baldwinlouie, I am out of town will resume work on it on Monday.

Hi @baldwinlouie, changes are done. Please review.

@shubham_jain, Thank you for the great patch. It looks good. I have three pieces of feedback. See the attached screenshot.

@shubham_jain,

Thanks for the patch. I have two pieces of feedback.

1. Instead of

  .ui-button .ui-icon {
    background-image: url(../../images/icon_white_close.png) !important;
    background-size: 12px;
    opacity: 1;
  }

I think it should be

  .ui-dialog-titlebar-close {
    background-image: url(../../images/icon_white_close.png) !important;
    background-size: 12px;
    opacity: 1;
  }

Without referencing .ui-dialog-titlebar-close, I do not see the "X" icon. What do you think?

2. See this small change for .btn-danger within the ui dialog.

Thanks again!

@shubham_jain Thanks for the update. A couple of issues.

This might be another issue, but this line causes a syntax error:
https://git.drupalcode.org/project/rigel/-/blob/38e150e1f231d4f90c6100ad...

Secondly, I still see the .btn-danger button with blue text . I see that your change addresses it using:

      &.use-ajax {
        color: $black;
      }

However, in my testing, the button doesn't have a .use-ajax class attached. Look at this screenshot. Are you seeing the same classes?

@shubham_jain, any questions after my last post?

Hi @baldwinlouie, I am out of town from last 5 days and will come back after two days.

After that will work on this issue.

Hi @baldwinlouie, I had to rebase the repository so took more time but resolved the issue.
Please review and verify.

@shubham_jain, Thank you for providing the update. It does fix the issue. Please check my comment above. https://git.drupalcode.org/project/rigel/-/merge_requests/43#note_214048

Those lines should not be removed. Can you please revert that? Btw, I don't see the corresponding change for those lines in the SCSS. Was that a manual change?

Hi @baldwinlouie , in order to close this issue I made a commit where I reverted the changes you mentioned.

I did a test and everything seems to work fine.

These changes have been committed and pushed to the issue fork for review and testing.

@tguerineau , thank you so much for fixing this patch. The fix works and looks good. Can I ask that you rebase the PR since things have changed. Once rebased, we will test one more time and then get it committed.

Hi @baldwinlouie ,

I've updated the branch with the latest changes from the 6.x branch as requested.

- Successfully rebased our branch onto the origin/6.x branch.
- Resolved all conflicts (if any) during the rebase process.
- Pushed the updated branch to the remote repository.

The branch is now up-to-date and ready for a final review and testing. Everything seems to work fine, please let me know if there's anything else that needs to be addressed.

@tguerineau, thank you for the updated patch! It looks good to me now.

Hi @baldwinlouie,

I have completed the rebase of the branch once again as requested. During the process, I encountered and resolved numerous conflicts to ensure that the latest changes from the 6.x branch are incorporated with the fixes intended for this issue.

Given the extensive nature of these conflicts, I would appreciate a thorough review to confirm that all intended functionality is intact and that no contributions have been inadvertently omitted.

The branch is now up-to-date. Please let me know if there's anything else that needs to be addressed.

@tguerineau thank you so much for rebasing and cleaning up the patch. I see two concerns

1. It seems like when hovering over a vertical fieldset, it is acting weird. (The text is jumping around when you hover over a fieldset). It's hard to describe, so I am attaching a screen recording of what I see in my environment.

2. The fieldset text in the screen recording should be left aligned and not center aligned.

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

Hello,

I have updated the fork and fixed the issue mentioned in #39. A new merge request !68 has been created.

Thank you.

@baldwinlouie

Can you please review the MR if you have a chance to take a look at it? Thanks!

@sachin.addweb, Thank you for taking over this patch. There are stylelint issues reported with main.css during Unit testing. Can you please run Step 6 from the README.md, npx prettier --config .prettierrc.json --write "assets/css/main.css". That will fix all the stylelint issues.

Hello @baldwinlouie,

I have resolved all the Stylelint issues. Could you please review them at your convenience?

Thanks!

Changing to needs review to trigger testing.

@sachin.addweb, Thank you for the updated patch. I made one code comment. Also, please see attached screenshot. When you added the new "X" as the close icon, there smaller one is still showing up.

@baldwinlouie

Thank you for your review. It helps us a lot.

@sachin.addweb

I found that you created your MR based on the drupal/rigel:6.x branch. The 6.x branch has been behind, but I was thinking it was fine since we will proceed with 7.0.x releases and 6.0.x will be obsolete in the future.

It explains the reason why your MR needed to be fixed for the StyleLint. Anyhow, thank you so much for fixing the standard coding errors on the 6.x branch.

In that case, could I ask you create the MR for drupal/rigel:7.x, and can you update both branch when you fix Baldwin's comment?

Thanks!

@sachin.addweb, Thank you for the updated patch. It fixes the "close" icon issue.

Please take a look at this comment: https://git.drupalcode.org/project/rigel/-/merge_requests/68#note_370892

Aside from that, it is looks good to me.

@baldwinlouie, As you had suggested, we have fixed the code. Kindly check it.

@yas, And as you mentioned that we need to work on drupal/rigel:7.x, we are currently working on that. We will update you later.

@sachin.addweb

Thank you for the update. I've changed the status to Needs review so that you can indicate the MR is ready to be reviewed. You can change the status by yourself in the future.

Thanks

@sachin.addweb. Thank you for the fixes to the patch. It looks good to me now.

@baldwinlouie

Thank you for your review.

@sachin.addweb

Thank you for the update. -I’ll merge the patch into 6.x; and change the status Needs work for the 7.x branch.

@sachin.addweb

Can you make MRs for drupal/rigel:7.x and 8.x branch? Thank you for your extra efforts.

@yas, I'll create the 7.x and 8.x branch

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