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

Improve login guidance and non authorized user experience #3407

Merged
merged 46 commits into from
Sep 27, 2021

Conversation

magicznyleszek
Copy link
Member

Description

Cleaner and better handling of non authorized user (or user without permissions) accessing different part of UI. Also improves behind the scenes of how we handle routes.

Additional details

Includes work from:

Related issues

Fixes #3318

# Conflicts:
#	jsapp/js/components/sectionNotFound.es6
# Conflicts:
#	jsapp/js/components/accountSettings.es6
@p2edwards p2edwards assigned p2edwards and unassigned duvld Aug 19, 2021
@p2edwards p2edwards self-requested a review August 19, 2021 15:56
# Conflicts:
#	jsapp/js/components/accountSettings.es6
#	jsapp/js/components/changePassword.es6
#	jsapp/js/components/common/accessDeniedMessage.es6
# Conflicts:
#	jsapp/js/components/formSubScreens.es6
#	jsapp/js/mixins.es6
# Conflicts:
#	jsapp/js/mixins.es6
#	jsapp/js/router/allRoutes.es6
Copy link
Contributor

@p2edwards p2edwards left a comment

Choose a reason for hiding this comment

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

  • accessDenied.es6 - There's a translated string that will not translate well.
  • I accidentally removed package-lock.json while fixing merge conflicts.

jsapp/js/components/bigModal/bigModal.es6 Show resolved Hide resolved
.access-denied__body {
max-width: 600px;

.access-denied__header {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this kind of nesting normal/necessary when using BEM with scss?

this isn't really specific to this branch, maybe it's something we could discuss in Zulip (or have already discussed)

Copy link
Member Author

Choose a reason for hiding this comment

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

So after the discussion we had on Zulip, I've updated https://github.com/kobotoolbox/kpi/wiki/Frontend-Development and un-nested those selectors

Comment on lines 41 to 51
t('Please ')
:
t('Please try logging in using the header button or ')
}

{envStore.data.support_url ?
<a href={envStore.data.support_url} target='_blank'>{t('contact the support team')}</a>
:
t('contact the support team')
}
{t(" if you think it's an error.")}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this translate well?

Copy link
Contributor

Choose a reason for hiding this comment

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

  • Please try logging in using the header button or [contact the support team] if you think it's an error
  • Please [contact the support team] if you think it's an error.

@dorey
Copy link
Contributor

dorey commented Sep 14, 2021

I'm getting this error and it is preventing the page from rendering. @p2edwards is not getting this error, so it could be something specific to my installation.

I have tried reinstalling node_modules

image

Does it look familiar to anyone?

@magicznyleszek
Copy link
Member Author

@dorey could this be because of missing reflux types in package-lock.json?

@dorey
Copy link
Contributor

dorey commented Sep 15, 2021

I don't think this is because missing types. It's possible the problem came from when @p2edwards and I merged beta into this branch. I'm not able to see a form's landing page.

Copy link
Contributor

@p2edwards p2edwards left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@dorey
Copy link
Contributor

dorey commented Sep 27, 2021

i think this is good to merge?

@p2edwards p2edwards merged commit 61ee14f into beta Sep 27, 2021
@p2edwards p2edwards deleted the 3318-login-guidance branch September 27, 2021 16:41
jdugh pushed a commit to geohyd/kpi that referenced this pull request Nov 21, 2022
Improve login guidance and non authorized user experience
jdugh pushed a commit to geohyd/kpi that referenced this pull request Nov 13, 2023
Improve login guidance and non authorized user experience
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.

4 participants