🇧🇷Brazil @RenatoG

New maintainer mandclu added and permissions updated

Welcome aboard

Actually the README exists on .txt format: https://git.drupalcode.org/project/memory_profiler/-/blob/2.0.x/README.t...

We can just modify to a markdown format if needed

Thank you so much for your contribution

Marking as needs review

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

Thanks everyone

Thanks for your contribution

Patch #2 is using Drupal:service() but this class has construct so it can use dependency injection

It feels like you are making code suggestions just to get a commit credit

@jrockowitz you're wrong

• D.O don't have "commit credit"
• Currently is "issue credit"
• So doesn't matter who is the commiter
• And personally speaking I definitely don't need this

Please slow down with your reviews and patches

Sorry, it's a democracy

• It's open-source project
• Everyone are free to make contributions
• If you think that isn't useful, you as maintainer can remove the credits on field "Credit & committing" @see Granting credit to issue contributors →
• But you can't block any user to contribute if he/she wants
• You should respect everyone
• I recommend you to read Drupal Code of Conduct →
• If you don't agree with CoF or project license, feel free to go out and create a private project where you can block people and do everything you want

Aware that #60 needs works according to comment above

On this meantime, for any reason if someone needs to use on 8.x-4.7 tag, this is the patch

P.S. Making as hidden since tests needs to be done development branch

Aware that #60 needs works according to comment above

On this meantime, for any reason if someone needs to use on 8.x-4.7 tag, this is the patch

P.S. Making as hidden since tests needs to be done development branch

Merged

Keeping open to receive new updates from bot if necessary

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

Merged into dev branch

P.S. I'm going to keep issue open as RTBC to receive new automatic patches from Project Update Bot

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

My pleasure

Hello!

README

This project allows you to identify the browser and the device used. A class will be added to the HTML tag.

Source: https://git.drupalcode.org/project/browser_detect

Project page

This project allows you to identify the browser and the device used.
A class is inserted in the HTML tag so you can use as you need.

Source: https://www.drupal.org/project/browser_detect →

Screenshot of example: https://www.drupal.org/files/project-images/img_4.png →

Is actually useful

• Part of PHP CodeSniffer - Command Line Usage →
• Patch was created a long time ago
• On that time the goal was to learn/ practice workflow on community
• Also contribute with small fixes
• Definitely don't impacts the functionality
• We can say that the Priority is Minor
• But valid since is a framework that follows standards
• Anyway, as a maintainer you may not accept it, and it doesn't matter
• Just like everyone can be ignorant
• Last but not least; useless is someone answer an issue from 7 years ago :)

Definitely a good catch! Thanks for reporting that @justcaldwell

Probably is a conflict in the bootstrap library

Explanation

• When you put Modal to appear on /admin/* it loads BS automatically on admin pages
• If your Modal isn’t /admin/* it don’t load
• When don’t load it works
• So probably is a conflict

If you want to make sure:

1. Open Modal Global Settings
2. Mark the settings to “don’t load bootstrap automatically”
3. Save
4. Put Modal in /admin/*
5. Verify the gear icon

P.S. it isn’t the final fix, is just a test to confirm that is a conflict on BS library

If you already has BS library being loaded in other project your Modal can works even disabling this option

Strange. Just tested on simplytest.me and it worked as screenshot →

Do you mind confirming that is the same icon, please?

@uv516 question #1 wasn't answered

1) Modal works without "module_filter"?

for now isn't critical:

• This case seems a conflict with other project
• Isn't impacting everyone
• 300+ sites using 7.x-1.3 without issues

1) Not "Modal filter", but "Module filter" (look at the pictures)

Was a typo, but you understood the question and didn’t answer that

Seems a conflict

Questions:

• 1) Modal works without "Modal filter"?
• 2) Do you already have bootstrap loaded before Modal Page installed? If yes, which version?

Thanks

Released: https://www.drupal.org/project/memory_profiler/releases/2.0.0-beta1 →

commited into 2.0.x. Ty

patch re-rolled

Needs reroll after 📌 Add the memory peak to the response header Needs review

Merged to the dev branch.

Thanks for your contribution

Adding in the roadmap for the next relese

committed into 2.0.x dev branch

Thanks everyone

Seems good. Adding into 🌱 Plan for Memory profiler 2.0.0 Active

Available at https://www.drupal.org/project/memory_profiler/releases/2.0.x-dev →

renatog → created an issue.

renatog → created an issue.

Thanks everyone

If an existing maintainer has not commented, move the issue to the Drupal.org Project Ownership queue, by editing the “Project” field on this issue

Moving to Project Ownership due to lack of response

Thanks for your contribution @mahtab_alam

Marking as Needs Works because MR status is:

Merge blocked: fast-forward merge is not possible. To merge this request, first rebase locally

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

Merged into the dev branch. Thanks a lot @mpaulo

Reviewed and seems good for me

I'm interested being a maintainer

Comment here explaining why you would like to maintain the module

Reasons

• Last commit +7 years ago
• Last stable release → for D7 only released +9 years ago
• +100 sites using according to usage statistics for Memory profiler →

Goals

• Release the new version with 📌 Drupal 10 compatibility Needs review
• Keep project updated and safe

confirm that you have security coverage opt-in permission

✓ Can opt projects into security advisory coverage ✅

Contributions

@edsoncarlos, @murilohp and I are working in an important project using memory_profiler.
List of contributions, activities and clean-up in the issue queue:

https://www.drupal.org/project/memory_profiler/issues/3366573 📌 Drupal 10 compatibility Needs review
https://www.drupal.org/project/memory_profiler/issues/3018392 📌 Dependency namespacing in .info.yml file RTBC
https://www.drupal.org/project/memory_profiler/issues/3366605 📌 Add the memory peak to the response header Needs review
https://www.drupal.org/project/memory_profiler/issues/3228272 📌 Add core_version_requirement: ^8 || ^9 for Drupal 9 Postponed
https://www.drupal.org/project/memory_profiler/issues/3141070 📌 Automated Drupal Rector fixes Closed: duplicate
https://www.drupal.org/project/memory_profiler/issues/3327996 💬 Offering to maintain Memory profiler Closed: duplicate

#2 makes sense, +1 to go

@larruda huge pleasure talking to you, master! 👑

any reason for being config entities?

To use configuration management

Explanation

1. Version 8.x-2.19 → is using Content Entity ( ref )
2. Migrated to Config Entity on 4.x at #3142963: [PLAN] Change Modal Entity from Content Entity to Config Entity →
3. Motivation was to export Modals with drush cex like webform
4. Was evaluated pros / cons of Content Entity x Config Entity
5. At the time we didn't find a "hard-reason" to use Content Entity
6. On the other hand, exporting feature was something interesting

For this reasons the migration but I'm open to consider using Content Entity if we have a good justification

About making Modal fieldable; I truly agree that would be an important milestone for the project's growth

thanks everyone

I’m happy to know that

So, closing the issue

After the upgrade have tried to run update.php and clear the caches?

Explanation

• The last version of 1.x is 1.3
• The last version of 2.x is 2.0.11

So this upgrade from 1.3 to 2.0.11 is the most common, and according to the project usage → , there are 21,616 using 2.x version, so it's curious that for them is working fine

I'd suggest:

1. Upgrade from 1.3 to 2.0.11
2. Run drush updb
3. Run drush cr

If didn't work, my recommendation as workaround:

1. Upgrade from 1.3 to 2.0.0
2. Run drush updb
3. Run drush cr
4. Upgrade from 2.0.0 to 2.0.11

Merged. Thanks everyone

Thanks @gautam_105

But I think the MR wasn't created correctly, right?

Please take a look

Aware about the explanation on #37

On this meantime, if someone needs a workaround for D10.x, this patch may help you

Actually I think it wasn’t fixed since the patch wasn’t committed, right?

Moving back the previous status

renatog → created an issue.

Updated /node/<ID> by the alias

Woow, thank you so much for this amazing contribution @jnlar

On my first look in the MR, seems good

I'm moving to "Needs Review" just to manual tests

The maximization feature is op-in and disabled by default, in ModalForm.php
Pre-existing/new modals will need to check 'Enable Maximize Button' and then save the edit modal form in order to have the button display in their modal.

That's perfect!
With that solution it won't impact the existent sites but the feature will be available and we'll let them know on release notes

I think the enable_modal_header dependency is sensible as it follows the same pattern as other buttons (close button) placed in the header region.

Agreed!
Since this button is located on Modal Header I think makes sense to include there

Thanks for the patch @bburg

Moving to Needs Review to test that and commit into 5.x branch

Appreciate

Awesome job @jnlar, thank you so much

I'm marking as "Needs Works" because if we merge that today, will appear this new button in all modals of 2k+ sites

So before merging that we need 2 thinks:

1. Create an option inside of Modal CMS for Enable Modal Maximization (Default "true")
2. Create a hook_update_N(): Load all existent Modals and set Enable Modal Maximization as false

With that it won't impact the existent sites, but if they want to use they'll be able to enable that on Modal entity via CMS. Is that makes sense?

P.S. if prefer we can create a separated issues for this

Committed the info files, so seems that on Drupal the project is ready but now we need to update the "Janrain REST" to be compatible: https://github.com/janrain-rest/janrain-rest

Any contributions will be welcome

Backing to NW since we need to update the info file

Committed to the dev branch and created a new release 4.1.0-beta1 → available for D10

Thanks

Seems good

Moving to 4.1.x-dev, with the plan to launch 4.1 version with D10 support

Awesome! Thanks @tguerineau , we’ll test it and provide feedbacks

I think we can implement the same improvement on this other method to be consistent

renatog → created an issue.

Moved to the dev branch. Thank you so much for your great job @tguerineau

Adjusted phpcs

It's working fine, just come points on phpcs

FILE: modal_page/src/Controller/ModalAjaxController.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 6 LINES
-------------------------------------------------------------------------------------------------------------------------------------
 101 | ERROR | [x] Namespaced classes/interfaces/traits should be referenced with use statements
 104 | ERROR | [ ] The array declaration extends to column 85 (the limit is 80). The array content should be split up over multiple
     |       |     lines
 112 | ERROR | [ ] The array declaration extends to column 84 (the limit is 80). The array content should be split up over multiple
     |       |     lines
 118 | ERROR | [ ] The array declaration extends to column 108 (the limit is 80). The array content should be split up over multiple
     |       |     lines
 130 | ERROR | [x] Expected newline after closing brace
 136 | ERROR | [ ] The array declaration extends to column 115 (the limit is 80). The array content should be split up over multiple
     |       |     lines
-------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------------------

Time: 959ms; Memory: 14MB

Moved to the dev branch

@kevinquillen thanks for your feedback. It's a pleasure having your presence with great suggestions in the issue queue. I hope to see more

Best

better name in the variable eventKey

Moved to the dev branch

@kevinquillen thanks for your feedback. It's a pleasure having your presence with great suggestions in the issue queue. I hope to see more

Best

Patch with the fix suggested on IS

New release 5.0.5 → available with this fix

Thanks for the patch @tguerineau, let's review the code and test it

Agreed! Thanks

Until this ticket is implemented as a workaround we can use config ignore → to disconsider the auto-complete configs

Thanks for your feedback @kevinquillen

No cookies are ever written when a user clicks "OK" or in this case re-labelled to "I Accept"

The project wasn't designed to create cookies on "OK" button, so isn't a "bug", but I definitely is a great suggestion so I'm converting to Feature Request

Workaround

For now as a temporary you can use HOOK_modal_ID_alter() and when the user clicks on "I accept" we can store the cookie programmatically

Example:

/**
 * Implements hook_modal_ID_alter().
 */
function HOOK_modal_ID_alter(&$modal, $modal_id) {

  // Set cookie to don't show again
  $cookieName = 'please_do_not_show_again_modal_id_' + $modal_id';

  // TODO: logic to insert the cookie.
}

Other possibility:

1. Edit this Modal(s)
2. Put a class on "OK button" E.g. i-accept-modal
3. Hide the checkbox please don't show again for the end-users with display:none
4. Create a custom JS to mark this checkbox when the user clicks on "I Accept" class

I also see no way to control the cookie name for each modal

We also don't have this option, I created a ticket to implement that ✨ Create a way to customize the cookie name for "don't show Modal" Active

Question: If we have one config inside "Global Settings" to customize the cookie is enough, or do you think is necessary to customize cookie name from Modal to Modal?

Is there any reason why the keydown event is not as simple as

I think there is no reason. Good catch btw, thanks for reporting @kevinquillen

renatog → created an issue.

Thanks for providing more information

Let’s try to reproduce

Moved to the dev branch

@tguerineau great suggestion of using JSON instead of "integer" on AJAX response. I created this separated issue 📌 Update the ModalAjaxController::hookModalSubmit() to return JSON instead of int value Fixed and we can work there

Thanks for reporting and providing feedbacks, we really appreciate

first problem: undefined method Drupal\Core\Extension\ModuleHandler::getImplementations() is the same of #3293221: D10: ModuleHandler::getImplementations() deprecation →
second one: Input value contains a non-scalar value is the same of 🐛 Ajax pager - Input value "viewsreference" contains a non-scalar value Fixed

This patch contains the same fixes

renatog → created an issue.

Are there any pre-requisites for using modal module?

No, there isn't

I installed it on my production site also but there also the page admin/structure/modal does not open

Strange, I just installed and this page is working for me - ( screenshot → )

P.S. I also tested changing the configs on "Global Settings" but continue working

Do you mind pasting the log of the error, please?

1) Go to /admin/modules and enable the "Database Logging"
2) Open the page "

/admin/structure/modal

" to see the "blank page"
3) Open the page: "/admin/reports/dblog"
4) Get the log of error and paste here, please

It'll helps in the investigation

New release 5.0.4 → is available with this fix

Committed to the dev branch

Thanks for your contribution @gawalin - good catch!

Tested the patch and it really works well

That's nice! Thanks for the patch, let's test it

I tested and worked for me

@qsurti as #2 could you please provide a "step by step" to reproduce, please?

@robbt , this one happens only on 5.0.3?

I'm wondering if this is side-effect of the new release

I saw that the description was in the "tags" field. So just moving to the description

P.S. If someone else can validate if is happening for you as well, please?

Make 2.x default branch on Gitlab

Marking as duplicate in favour of 📌 Set default branch to 2.0.x Active

Thanks everyone

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

Thanks everyone

The new release https://www.drupal.org/project/page_manager_search/releases/2.0.0-beta5 was created at 🌱 Plan to 2.0.0-beta5 Fixed and it's available with this fix

Release available at https://www.drupal.org/project/page_manager_search/releases/2.0.0-beta5 →

renatog → created an issue.