Skip to content

[Improvement] Implement design theme grid on ui component#14470

Merged
magento-engcom-team merged 10 commits intomagento:2.3-developfrom
simpleadm:design-theme-grid-ui-component
Jul 2, 2018
Merged

[Improvement] Implement design theme grid on ui component#14470
magento-engcom-team merged 10 commits intomagento:2.3-developfrom
simpleadm:design-theme-grid-ui-component

Conversation

@simpleadm
Copy link
Copy Markdown
Contributor

@simpleadm simpleadm commented Mar 30, 2018

Description

Several versions ago backend grid widgets (\Magento\Backend\Block\Widget\Grid) were marked as
deprecated in favour of UI component implementation. This PR refactors design theme grid to use UI components.
Grid location: Content > Design > Themes

At least it will save time of third party developers who needs to extend themes grid :)

Manual testing scenarios

Before changes:
image

After changes:
image

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@VladimirZaets
Copy link
Copy Markdown
Contributor

Hi @simpleadm , any updates?

@simpleadm
Copy link
Copy Markdown
Contributor Author

Hi @VladimirZaets ,
what updates you are waiting for?

@VladimirZaets
Copy link
Copy Markdown
Contributor

Hi @simpleadm , we have 2 functional tests that were failed with your changes. That's are Magento\Widget\Test\TestCase\CreateWidgetsEntityTest.test with data set "CreateWidgetEntityTestWithDifferentThemes_0"
and Magento\Widget\Test\TestCase\DeleteWidgetEntityTest.test with data set "DeleteWidgetEntityTestVariation1_0".
Can you look and fix them, please?
If you need some help or guide how to running functional tests locally, please let us know.

@VladimirZaets
Copy link
Copy Markdown
Contributor

@magento-engcom-team give me new test instance

@magento-engcom-team
Copy link
Copy Markdown
Contributor

Hi @VladimirZaets. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Copy Markdown
Contributor

Hi @VladimirZaets, here is your new Magento instance.
Admin access: https://pr-14470.engcom.dev.magento.com/admin
Login: admin Password: 123123q
Please make sure you are PR author or assignee to access the instance.

@simpleadm
Copy link
Copy Markdown
Contributor Author

Hi @VladimirZaets ,
Please check updates.

* Theme grid collection
*/
class Collection extends \Magento\Theme\Model\ResourceModel\Theme\Collection
class Collection extends ThemeCollection implements SearchResultInterface
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @simpleadm, please make some refactoring:

  1. The collection shouldn't extend the ThemeCollection collection, it should only implement SearchResultInterface.
  2. Use ThemeCollection to getting access to data inside implemented methods.

@okorshenko okorshenko modified the milestones: April 2018, May 2018 May 18, 2018
@simpleadm
Copy link
Copy Markdown
Contributor Author

Hi @VladimirZaets ,
Please check new changes.

@VladimirZaets
Copy link
Copy Markdown
Contributor

Hi @simpleadm, thanks, I took this PR in progress.

@VladimirZaets
Copy link
Copy Markdown
Contributor

Hi @simpleadm. The grid rows are not clickable, consequently, edit page is not available too.
Please fix it. Thank

@simpleadm
Copy link
Copy Markdown
Contributor Author

Hi @VladimirZaets ,
View action was added, please check.

@VladimirZaets
Copy link
Copy Markdown
Contributor

Hi @simpleadm.
We tried to test your fix and found the next problem:
View action redirects to Dashboard when Add Secret Key to URLs=Yes

@magento-cicd2
Copy link
Copy Markdown
Contributor

magento-cicd2 commented May 31, 2018

CLA assistant check
All committers have signed the CLA.

@simpleadm
Copy link
Copy Markdown
Contributor Author

Hi @VladimirZaets ,
Url was fixed.

@magento-engcom-team
Copy link
Copy Markdown
Contributor

Hi @simpleadm. Thank you for your contribution.
We will aim to release these changes as part of 2.3.0.
Please check the release notes for final confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants