-
-
Notifications
You must be signed in to change notification settings - Fork 421
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
refactor(functions): issue 4134 blocking user with incomplete profile… #4144
base: master
Are you sure you want to change the base?
Conversation
onearmy-community-platform
|
Project |
onearmy-community-platform
|
Branch Review |
pull/4144
|
Run status |
|
Run duration | 05m 16s |
Commit |
|
Committer | Venu G Soganadgi |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
1
|
|
0
|
|
0
|
|
78
|
View all changes introduced in this branch ↗︎ |
This reverts commit 06849b8.
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.
Nice one, thanks @V24039 and sorry for the delay getting back to you.
One style comment left. The only other thing is if you could please add this to the cypress tests. The settings tests were temporary commented out but they're back now.
On packages/cypress/src/integration/settings.spec.ts
there's settings -> Fixing Fashion -> Can create space. Maybe just after cy.get('[data-cy=incompleteProfileBanner]').click()
is called, go to the map tab, expect your new component to be visible. Then in the step 'Can add map pin' you expect your new component not to be visible?
<Alert | ||
variant="info" | ||
sx={{ | ||
marginTop: 2, | ||
display: 'flex', | ||
flexDirection: 'column', | ||
gap: 2, | ||
alignItems: 'flex-start', | ||
}} | ||
> |
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.
Rather than an alert, I think it's best to use <Card variant=borderless">
as your starting point and format from there. Should be less rules to set then.
… to add map pin
PR Checklist
PR Type
What kind of change does this PR introduce?
What is the current behavior?
User with empty profile can add map pin
What is the new behavior?
User with empty profile cannot add map pin and message to complete profile and a complete profile button is shown
Does this PR introduce a DB Schema Change or Migration?
Git Issues
Closes #4134
What happens next?
Thanks for the contribution! We try to make sure all PRs are reviewed ahead of our monthly maintainers call (first Monday of the month)
If the PR is working as intended it'll be merged and included in the next platform release, if not changes will be requested and re-reviewed once updated.
If you need more immediate feedback you can try reaching out on Discord in the Community Platform
development
channel.