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

docs: reorganize (part 2) #841

Merged
merged 45 commits into from Sep 22, 2023
Merged

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Sep 19, 2023

Description
The current documentation is difficult to understand where it is written.
See #839

  • add section "User Management"
  • add pages
  • move files to the section directories
  • tweaks
Screenshot 2023-09-19 10 53 02

Online Demo: https://datamweb.github.io/shield/

Checklist:

  • Securely signed commits
  • [] Component(s) with PHPDoc blocks, only if necessary or adds value
  • [] Unit testing, with >80% coverage
  • User guide updated
  • [] Conforms to style guide

@kenjis kenjis added the documentation Improvements or additions to documentation label Sep 19, 2023
@kenjis kenjis marked this pull request as draft September 19, 2023 01:36
@kenjis kenjis marked this pull request as ready for review September 19, 2023 01:57
Copy link
Collaborator

@datamweb datamweb left a comment

Choose a reason for hiding this comment

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

Generally good.
Only I prefer to have section acknowledgements in these documents.

Comment on lines 157 to 160

If you get `Specified key was too long` error:

1. Use InnoDB, not MyISAM.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
If you get `Specified key was too long` error:
1. Use InnoDB, not MyISAM.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it is necessary now with the changes applied in the migration file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

6. Configure **app/Config/Email.php** to allow Shield to send emails.

```php
<?php
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it(<?php) added to all existing codes?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a whole file of it. So it seems better to have <?php.
I don't think <?php should be added to all existing codes.

# Authentication Flow
# Using Session Authenticator

**Session** authenticator provides traditional Email/Password authentication.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Email/Password It's probably a term, but my understanding of this at first glance is that Shield only supports email/password (username/password, mobile/password, etc.).

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to ID/Password.

@@ -27,7 +29,7 @@ public array $redirects = [

### Configure Remember-me Functionality

Remember-me functionality is enabled by default for the `Session` authenticator. While this is handled in a secure manner, some sites may want it disabled. You might also want to change how long it remembers a user and doesn't require additional login.
Remember-me functionality is enabled by default. While this is handled in a secure manner, some sites may want it disabled. You might also want to change how long it remembers a user and doesn't require additional login.
Copy link
Collaborator

Choose a reason for hiding this comment

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

which one? Remember me
or Remember-me.
please choose one everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Remember-me. Done.

Comment on lines 34 to 35
> **Note**
> These filters are already loaded for you by the registrar class located at **src/Config/Registrar.php**.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I recently saw a video of an instructor resetting the filters(app/filters.php). Therefore, I prefer to add this issue in line 3.

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved the note up.

# Magic Link Login

Magic Link Login is a feature that allows users to log in if they forget their
password.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Or use as a one-time password.

Copy link
Member Author

Choose a reason for hiding this comment

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

Using Magic Link Login as a one-time password authentication?
I don't know if it is supposed to be used for anything other than forgotten passwords.

@kenjis kenjis changed the title docs: reorganize (part 2/2) docs: reorganize (part 2) Sep 19, 2023
@kenjis
Copy link
Member Author

kenjis commented Sep 19, 2023

Added Acknowledgements.

@kenjis kenjis merged commit 6be3162 into codeigniter4:develop Sep 22, 2023
1 check passed
@kenjis kenjis deleted the docs-reorganize-2 branch September 22, 2023 08:45
@kenjis kenjis mentioned this pull request Sep 22, 2023
2 tasks
@kenjis kenjis added the enhancement New feature or request label Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants