Problem/Motivation

Currently we're using bootstrap to show Modal. Should be amazing using Drupal Core Dialog API to show that and remove dependency of BS

Proposed resolution

Implement that using Dialog API: https://www.drupal.org/docs/drupal-apis/ajax-api/ajax-dialog-boxes

Issue fork modal_page-3281106

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

RenatoG created an issue. See original summary.

zeshan.ziya made their first commit to this issue’s fork.

zeshan.ziya’s picture

zeshan.ziya commented

@RenatoG I have started working on this.

renatog’s picture

renatog commented

Wooow awesome! Thanks a lot!

marcusvsouza’s picture

marcusvsouza commented

Are you working on thi issue?
Just to know, if you don't, I really like to work on this one!
Thanks!

renatog’s picture

renatog commented

Are you working on this issue?
Just to know, if you don't, I really like to work on this one!
Thanks!

Hello @marcusvsouza! Thanks a lot for that. I'm not working on this because of lack of time =/

So feel free. Any merge request or patches will be welcome!
:D

marcusvsouza’s picture

marcusvsouza commented
Assigned: Unassigned » marcusvsouza

Working on it!

zeshan.ziya’s picture

zeshan.ziya commented

Thanks @marcusvsouza, I got busy with some work so didn't get time to work on this. Please let me know if you need any help from my side.

marcusvsouza’s picture

marcusvsouza commented

@zeshan.ziya
Thank you for your availability, I will definitely reach you if I need help!

marcusvsouza opened merge request !23

marcusvsouza’s picture

marcusvsouza commented

I've made progress on the task and made a MR to show it, I'm still working on the issue, any suggestions for improvement or changes to what I've done so far are welcome.

eduardo morales alberti’s picture

eduardo morales alberti
Primary language Spanish
Location Spain, 🇪🇺
commented

I am interested on it, because my main theme has bootstrap 4 and the admin theme (GIN) does not have bootstrap.

jeffc518’s picture

jeffc518 commented

Thanks @marcusvsouza - I'll check out your merge request. Bootstrap is great if your custom theme is already using it, but adding it to the front end globally causes a number of conflicts on my end. I haven't seen much movement on this in awhile so wanted to make a note. If I have some time I'll see what I can contribute.

renatog’s picture

renatog commented

Bootstrap is great if your custom theme is already using it, but adding it to the front end globally causes a number of conflicts on my end

agreed, using Drupal Dialog API without dependency of BS would be amazing

If I have some time I'll see what I can contribute

Awesome! Really appreciated @jeffc518

chriseikelboom’s picture

chriseikelboom commented

@jeffc518 I'm also very interested in this change for the same reasons.

zeshan.ziya’s picture

zeshan.ziya commented

@renatog, I haven't seen an update in a while, so I'll try to spend some time on it and get it going. I see that v5 is now available. Do you think we should make it work directly with v5, or continue with 4.x?

renatog’s picture

renatog commented
Version: 4.1.x-dev » 5.0.x-dev

Thank you so much @zeshan.ziya

Do you think we should make it work directly with v5, or continue with 4.x?

v5 is better

We'll focus updates on 5.x so makes sense use this version

zeshan.ziya’s picture

zeshan.ziya commented

Hi @renatog, I am almost done with the changes. I am doing some final testing. I need help opening the merge request. I don't see an option to open the merge request. Can you please help with the same?

zeshan.ziya opened merge request !42

zeshan.ziya’s picture

zeshan.ziya commented

Hi @renatog, I have raised an MR. Please review. Please let me know if any changes are required.

renatog’s picture

renatog commented
Assigned: marcusvsouza » Unassigned
Status: Active » Needs review

Wow! Seems awesome! Thank you so much @zeshanziya

Moving to NR so we can validate. Anyone are welcome to help testing as well

If it works fine we can create 5.1.0-beta1 with this

zeshan.ziya changed the visibility of the branch 3281106-implement-drupal-dialog to hidden.

alfattal’s picture

alfattal commented

I tried to apply the MR as a local patch, but it failed to apply.

zeshan.ziya’s picture

zeshan.ziya commented

Hey @alfattal, thanks for giving it a try. I don’t have much experience with patching, but here’s what worked for me.

1. Download the diff from https://git.drupalcode.org/project/modal_page/-/merge_requests/42.diff.
2. Apply the patch with: `patch < 42.diff`

alfattal’s picture

alfattal commented

@zeshan.ziya Sorry for the late response. I've successfully applied the patch from the PR and I can confirm that it is working properly. 1 for RTBC!

renatog’s picture

renatog commented
Status: Needs review » Reviewed & tested by the community

I can confirm that it is working properly. 1 for RTBC!

Great! Thanks for testing

Moving that to RTBC to be merged soon

eduardo morales alberti’s picture

eduardo morales alberti
Primary language Spanish
Location Spain, 🇪🇺
commented

Any news? we tested it, and works properly.

renatog’s picture

renatog commented

Sorry, I didn't have time to merge that before but it's on my plans

I'll test that and merge as soon as possible

renatog’s picture

renatog commented

Any news? we tested it, and works properly

You're right. Tested and confirmed that it works fine

  • renatog committed 5fd8ba56 on 6.0.x authored by zeshan.ziya 
    Issue #3281106 by zeshan.ziya, marcusvsouza, renatog, eduardo morales...
renatog’s picture

renatog commented
Version: 5.0.x-dev » 6.0.x-dev
Status: Reviewed & tested by the community » Fixed

Merged to the dev branch: 6.0.x

Since it's a considerable milestone we're going to create a beta-version based on 6.x and keep the stable on 5.x

After a time when it's tested by users, we'll make 6.x stable

Thanks everyone, it's amazing

Status: Fixed » Closed (fixed)

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