Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DEP Upgrade front-end build stack #1032

Merged

Conversation

GuySartorelli
Copy link
Member

See silverstripe/silverstripe-admin#1389 for a wider discussion on most of the changes

Parent issue

Comment on lines -33 to +35
preview.entwine('ss.preview')._loadUrl(preview.find('iframe').attr('src'));
if (preview && typeof preview.entwine === 'function') {
preview.entwine('ss.preview')._loadUrl(preview.find('iframe').attr('src'));
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Required for testing purposes, where we don't actually have jquery with entwine in the DOM.

@@ -0,0 +1,6 @@
{
Copy link
Member Author

@GuySartorelli GuySartorelli Jan 13, 2023

Choose a reason for hiding this comment

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

See silverstripe/silverstripe-asset-admin#1320 (comment) for explanation of this file

Comment on lines +4 to +9
jest.mock('isomorphic-fetch', () =>
() => Promise.resolve({
json: () => ({}),
})
);

Copy link
Member Author

Choose a reason for hiding this comment

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

Taken from TreeDropdownField-test.js to avoid ReferenceError: TextEncoder is not defined when running jest tests. See this stackoverflow question for more details about this error and its causes.

@GuySartorelli
Copy link
Member Author

Failing unit test is unrelated.

@GuySartorelli GuySartorelli force-pushed the pulls/5/frontend-build-stack branch from 8a7ccd4 to b627a86 Compare January 17, 2023 23:51
Then I should see a "Saved 'Charlie's Block' successfully" notice
Then I should see a "Saved 'Charlie's Block' successfully" success toast
Copy link
Member Author

Choose a reason for hiding this comment

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

The "I should see some notice" fixture was removed, as it had previously been deprecated. See silverstripe/silverstripe-framework#10594

@GuySartorelli GuySartorelli force-pushed the pulls/5/frontend-build-stack branch from b627a86 to 7a84632 Compare January 18, 2023 01:16
@@ -4,14 +4,11 @@
use DNADesign\Elemental\Models\BaseElement;
use DNADesign\Elemental\Models\ElementalArea;
use DNADesign\Elemental\Models\ElementContent;
use SilverStripe\BehatExtension\Context\FixtureContext as BaseFixtureContext;
use SilverStripe\CMS\Tests\Behaviour\FixtureContext as BaseFixtureContext;
Copy link
Member Author

Choose a reason for hiding this comment

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

Necessary to include the anchor dropdown fixture method - we can't just add the SilverStripe\CMS\Tests\Behaviour\FixtureContext class to the fixtures in behat.yml because it has the same superclass as this one, which causes behat to fail since it doesn't know whether to call methods from this fixture class or the CMS one.
Using the CMS fixture class as the superclass here resolves that problem.

@GuySartorelli GuySartorelli force-pushed the pulls/5/frontend-build-stack branch 3 times, most recently from 7b6d468 to 3b3ec46 Compare January 18, 2023 21:42
Comment on lines -23 to -26
Examples:
| group |
| ADMIN |
| AUTHOR |
Copy link
Member Author

Choose a reason for hiding this comment

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

The scenario outline wasn't actually using these variables. See the docs on scenario outlines
There's no reason to use admin here anyway so it makes more sense to just remove the examples and change the outline to a normal scenario.

When I press the "tab" key globally
Then I should not see "Duplicate"
And I should see a ".tox-tinymce" element
Copy link
Member Author

Choose a reason for hiding this comment

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

There was an issue where the block expansion was being affected by this menu button

When I press the "space" key globally
Then I should see "Duplicate"
And I should see a ".tox-tinymce" element
When I press the "escape" key globally
Copy link
Member Author

Choose a reason for hiding this comment

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

An update to bootstrap and/or reactstrap has changed the behaviour to give focus to the first element in the dropdown when opening with the keyboard, so we need to use escape to close it rather than hitting space/enter again.

@GuySartorelli
Copy link
Member Author

Behat passes locally - I'll set up a full kitchen sink run (link in the main issue) with all of the PRs running all of the behat to show whether this actually passes with all of the changes applied.

@GuySartorelli GuySartorelli force-pushed the pulls/5/frontend-build-stack branch from 3b3ec46 to bfed1ab Compare January 26, 2023 02:26
@emteknetnz emteknetnz merged commit 24546c0 into silverstripe:5 Jan 29, 2023
@emteknetnz emteknetnz deleted the pulls/5/frontend-build-stack branch January 29, 2023 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants