-
Notifications
You must be signed in to change notification settings - Fork 27
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
add fix for redirect #1997
add fix for redirect #1997
Conversation
4650b01
to
2390be8
Compare
2390be8
to
7fb4f1d
Compare
36c0443
to
4bd3868
Compare
f235341
to
bfa7177
Compare
bfa7177
to
75cbab7
Compare
75cbab7
to
6a6a574
Compare
@vgalatro I made the fixes for the style linting and also fix the test. Looks like the project is missing the storybook module and also the stylelint is failing on a |
Yeah storybook is currently not working. The version we're using won't run on correctly on the newer versions of node and upgrading to the latest version of Storybook is a large lift that we're still looking into. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@accbjt , just one question about a configuration change, otherwise everything is looking good.
.eslintrc.js
Outdated
@@ -29,6 +29,7 @@ module.exports = { | |||
}, | |||
ecmaVersion: 2020, | |||
sourceType: "module", | |||
requireConfigFile: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason behind this configuration change? I'm not seeing a difference in results with the linter with or without this setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my computer it was giving me an error and wouldn't lint my files. Not sure why it was doing it but this is what fixed it. When I remove this line of code I get this error.
Parsing error: No Babel config file detected for /Users/tranb2/Sites/wpmedia/arc-themes-blocks/blocks/identity-block/components/login/index.jsx. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files.eslint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a babel config at the root of the project so that's strange it couldn't be found. Could you revert this line and push it back up (in both PRs please)? After that I can approve and merge both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vgalatro Yeah, I was thinking the same thing. Maybe it is a VSCode issue. I updated both PR's. Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
2019c2b
into
arc-themes-release-version-2.3.0
* add fix for redirect * add regwall fix more mobile * update PB preview query param * remove requireConfigFile config
* add fix for redirect * add regwall fix more mobile * update PB preview query param * remove requireConfigFile config
* add fix for redirect * add regwall fix more mobile * update PB preview query param * remove requireConfigFile config
add fix for redirect (#1997) * add fix for redirect * add regwall fix more mobile * update PB preview query param * remove requireConfigFile config
If you have not filled out the checklist below, the pr is not ready for review.
Description
Jira Ticket
Acceptance Criteria
Test Steps
Log in Redirect:
RegWall button:
Effect Of Changes
Before
Login Redirect:
RegWall:
After
Login Redirect:
RegWall:
Dependencies or Side Effects
None
Author Checklist
The author of the PR should fill out the following sections to ensure this PR is ready for review.
npm run lint
to check for errors. Often,npm run lint:fix
will fix those errors and warnings.npm run test:coverage
to see your progress.npm run test
, made sure all tests are passingplease explain why (so that we can fix it whenever it gets refactored).
Reviewer Checklist
The reviewer of the PR should copy-paste this template into the review comments on review.