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

Modal to show username/email advice before sending users to Wikipedia #5564

Conversation

ujjwalpathaak
Copy link
Contributor

Refs #3295

What this PR does

I have tried using two different approach for both jsx component and server componet.
This adds an intermediate state between the process of redirecting users to create a new account.

If this looks good I can work further on this issue as it would need more changes in terms of the tests and overall functionality.

Sample

Screencast.from.26-12-23.04.51.59.PM.IST.webm
Screencast.from.25-12-23.10.06.39.PM.IST.webm

@ujjwalpathaak ujjwalpathaak marked this pull request as draft December 26, 2023 12:01
@ujjwalpathaak
Copy link
Contributor Author

I tried running the CI with --headless=new argument as mentioned in #5565 by @gabina which fixes the 317 failing tests -- worked.
I guess there are no tests written to check the login process? as I see no conflicts after adding the middlewear screen.

Also @ragesoss can I gen an invite for the slack channel?

@ujjwalpathaak
Copy link
Contributor Author

Currently the model is looking like this -
Screencast from 30-12-23 02:53:05 PM IST.webm

@ujjwalpathaak ujjwalpathaak marked this pull request as ready for review January 2, 2024 17:25
@ujjwalpathaak
Copy link
Contributor Author

What suggestions should i put in?

@ragesoss
Copy link
Member

ragesoss commented Jan 2, 2024

Here's a quick draft:

Username rules:
* do not choose names which may be offensive, misleading, disruptive, or promotional.
* The username should represent one person; do not use your organisation's name.

Additional advice:

We encourage you to use an anonymous username, not one that includes your real name or other identifying information.

@ujjwalpathaak
Copy link
Contributor Author

image

@ujjwalpathaak
Copy link
Contributor Author

@ragesoss this PR is ready for review. Also can I get an invite for Slack?

@ragesoss
Copy link
Member

ragesoss commented Jan 3, 2024

@ragesoss this PR is ready for review. Also can I get an invite for Slack?

Yes, please let me know your email address (or email me at sage at wikiedu.org)

@ujjwalpathaak
Copy link
Contributor Author

email -> [email protected]

The examples failing doesn't seem to be related to my changes -
Failed examples:
rspec ./spec/features/assigned_articles_spec.rb:19 # Assigned Articles view lets users submit feedback about articles
rspec ./spec/features/namespace_stats_spec.rb:27 # Namespace-specific stats generates and renders stats for Cookbook namespace on en.wikibooks.org

@ujjwalpathaak
Copy link
Contributor Author

Screencast.from.04-01-24.03.28.48.AM.IST.webm

WikiEd Config -
Screencast from 04-01-24 03:21:56 AM IST.webm

@ragesoss
Copy link
Member

ragesoss commented Jan 3, 2024

New videos look perfect!

@ujjwalpathaak ujjwalpathaak requested a review from ragesoss January 3, 2024 22:09
import NewAccountButton from './new_account_button';
import PropTypes from 'prop-types';

const AdviceModal = ({ setModalShown, course, passcode, user }) => {
Copy link
Member

Choose a reason for hiding this comment

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

I suggest using a more specific name like 'UsernameAdviceModal'.

@ragesoss
Copy link
Member

ragesoss commented Jan 5, 2024

One thing I didn't notice until testing it locally is a UI bug from when you converted .icon-close-small. It used to be aligned in the upper right, and now the close button is on the left above the header. (This might also affect .icon-close; I haven't looked for where that gets used.) I'm going to go ahead and merge this PR. Let me know if you'd like to add a fix for the close buttons, otherwise I can do it.

@ragesoss ragesoss merged commit f090200 into WikiEducationFoundation:master Jan 5, 2024
1 check passed
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