Campinas - Brazil
Account created on 7 June 2022, over 2 years ago
  • Developer at CI&TΒ 
#

Merge Requests

Recent comments

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil
πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Hello @vedia it's not necessary to be a confirmed user to post past or open MR, you can follow the how to contribute guide β†’ .

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Thanks, @RenatoG for contribution.

The license now is correct.

Moving to RTBC.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Agree with @Wim Leers in #3, removed the $this->(), NR.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

diegors β†’ made their first commit to this issue’s fork.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Thanks, @RenatoG for contribution.

The license now is correct.

Moving to RTBC.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

@amarshkhl09 I tested in my local env using a test.hml page.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Removed the CSS selector, I am not sure if this approach is the best, I did some tests and the download is normal. Please review.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

diegors β†’ created an issue.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

diegors β†’ made their first commit to this issue’s fork.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

The documentation is great for me, reviewed, and the filer follows the README.md template β†’ . Moving to RTBC.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Did the rebase, MR is mergeable.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Fixed the tests, still need FE part as suggested in #22

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

diegors β†’ made their first commit to this issue’s fork.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

I think this issue can be closed as won't fix.

Running the test on modal_page 5.0.x, no errors found:

PHPUnit 9.5.28 by Sebastian Bergmann and contributors.

Testing /app/web/modules/contrib/modal_page/tests
.....................                                             21 / 21 (100%)

Time: 00:22.901, Memory: 16.00 MB

OK (21 tests, 50 assertions)
πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Working on it.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

diegors β†’ made their first commit to this issue’s fork.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

I could not reproduce the bug, tested using Drupal 9.5.2 and Olivero theme using fixed and Resizing iframe, both of the scrolled to the top.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

I did some changes, In file tests/valid_path.test in:

/**
   * Protected property of created order.
   *
   * @var mixed
   */
  protected $order;

I am not sure about the @var mixed, I think it should be a specific type.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Did some changes, needs review.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Applied the patch and works.

Before patch:

After patch:

Moving to RTBC.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

It was a small change, just check if file is new before validate.

Please review.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Add a return checkProtectedPage() method and send a HTTP_NO_CONTENT in response fixed the problem, please review.

πŸ‡§πŸ‡·Brazil diegors Campinas - Brazil

Fixed the tests and some coding standard errors in test file.

Still need review.

Production build 0.71.5 2024