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.

Sign up for GitHub

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

Jump to bottom

Fixes notification width on Students page #5278

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Fixes notification width on Students page #5278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions assets/css/admin-custom.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,7 @@ div.question_boolean_fields { margin-bottom: 10px; }
* Sensei custom navigation
*/
.sensei-custom-navigation {
display: flex;
flex-flow: column wrap;
align-items: flex-start;
gap: 20px;
display: grid;
Copy link
Member

Choose a reason for hiding this comment

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

My suggestion here would be to keep the previous code, because I think it would be better to move the notifications outside of the navigation. That way we won't have to account for them being there. See the next comments for more info.

Copy link
Member

Choose a reason for hiding this comment

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

Forgot to mention that the grid view changes how the navigation looks on the other screen (some of the spacing is missing), but we won't have to worry about that if we decide to move the notifications outside.

margin: 30px auto 20px;
}

Expand Down
22 changes: 11 additions & 11 deletions includes/admin/class-sensei-learner-management.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ public function __construct( $file ) {
add_action( 'wp_ajax_remove_user_from_post', array( $this, 'remove_user_from_post' ) );
add_action( 'wp_ajax_reset_user_post', array( $this, 'reset_user_post' ) );
add_action( 'wp_ajax_sensei_json_search_users', array( $this, 'json_search_users' ) );

// Add custom navigation.
add_action( 'in_admin_header', [ $this, 'add_custom_navigation' ] );
}
Expand All @@ -123,7 +122,7 @@ public function add_custom_navigation() {
if ( ! $screen ) {
return;
}

Copy link
Member

Choose a reason for hiding this comment

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

I think the empty space here makes the linter angry. 😄 Could you remove it?

if ( in_array( $screen->id, [ 'course_page_sensei_learners' ], true ) && ( 'term' !== $screen->base ) ) {
$this->display_students_navigation( $screen );
}
Expand All @@ -136,17 +135,18 @@ public function add_custom_navigation() {
*/
private function display_students_navigation( WP_Screen $screen ) {
?>
<div id="sensei-custom-navigation" class="sensei-custom-navigation">
<div class="sensei-custom-navigation__heading-with-info">
<div class="sensei-custom-navigation__title">
<h1><?php esc_html_e( 'Students', 'sensei-lms' ); ?></h1>
<div id="sensei-custom-navigation" class="sensei-custom-navigation">
<div class="sensei-custom-navigation__heading-with-info">
<div class="sensei-custom-navigation__title">
<h1><?php esc_html_e( 'Students', 'sensei-lms' ); ?></h1>
</div>
<div class="sensei-custom-navigation__separator"></div>
<a class="sensei-custom-navigation__info" target="_blank" href="https://senseilms.com/documentation/student-management?utm_source=plugin_sensei&utm_medium=docs&utm_campaign=student-management">
<?php echo esc_html__( 'Guide To Student Management', 'sensei-lms' ); ?>
</a>
</div>
<div class="sensei-custom-navigation__separator"></div>
<a class="sensei-custom-navigation__info" target="_blank" href="https://senseilms.com/documentation/student-management?utm_source=plugin_sensei&utm_medium=docs&utm_campaign=student-management">
<?php echo esc_html__( 'Guide To Student Management', 'sensei-lms' ); ?>
</a>
<hr class="wp-header-end">
Comment on lines +138 to +148
Copy link
Member

@m1r0 m1r0 Jul 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

What do you think of moving the .wp-header-end element away from the navigation (reverting this change) and directly to the students inner page? My idea is to move it right after where the navigation would end up after it's moved by the JS code. I think in this case the element could be moved in these two files:

Let me know if you have other ideas!

</div>
</div>
<?php
}

Expand Down