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

Make admin nav responsive #2241

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
27 changes: 26 additions & 1 deletion classes/controllers/FrmFormsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -1866,11 +1866,36 @@ public static function process_bulk_form_actions( $errors ) {
return $errors;
}

/**
* Includes html that shows a message when the device is too small to use Formidable Forms admin pages.
*
* @since x.x
* @return void
*/
private static function include_device_too_small_message() {
?>
<div class="frm_small_device_message_container">
<div class="frm_small_device_message">
<svg width="32" height="49" viewBox="0 0 32 49" fill="none" xmlns="http://www.w3.org/2000/svg">
Copy link
Contributor

Choose a reason for hiding this comment

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

Please optimize this SVG using https://jakearchibald.github.io/svgomg/

<rect x="1" y="1" width="30" height="47" rx="3" fill="#F5FAFF" stroke="#4199FD" stroke-width="2"/>
<rect x="12" y="5" width="8" height="2" rx="1" fill="#C0DDFE"/>
<path d="M23 33C23 29.6863 20.0899 27 16.5 27C12.9101 27 10 29.6863 10 33" stroke="#80BBFE" stroke-width="1.5"/>
<circle cx="10.5" cy="20.5" r="1" fill="#80BBFE" stroke="#80BBFE"/>
<circle cx="21.5" cy="20.5" r="1" fill="#80BBFE" stroke="#80BBFE"/>
</svg>
<b><?php esc_html_e( 'More on bigger devices', 'formidable' ); ?></b>
<p><?php esc_html_e( 'For the best experience, we recommend using Formidable Forms on larger devices such as a desktop or tablet.', 'formidable' ); ?></p>
<a href="javascript:history.back()" class="frm-button-primary"><?php esc_html_e( 'Go back', 'formidable' ); ?></a>
</div>
</div>
<?php
}

public static function route() {
$action = isset( $_REQUEST['frm_action'] ) ? 'frm_action' : 'action';
$vars = array();
FrmAppHelper::include_svg();

self::include_device_too_small_message();
if ( isset( $_POST['frm_compact_fields'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Missing
FrmAppHelper::permission_check( 'frm_edit_forms' );

Expand Down
1 change: 0 additions & 1 deletion classes/views/frm-forms/edit.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
?>
<div id="frm_builder_page" class="frm_wrap">
<div class="frm_page_container">

<?php
require FrmAppHelper::plugin_path() . '/classes/views/frm-forms/mb_insert_fields.php';
FrmAppHelper::get_admin_header(
Expand Down
99 changes: 86 additions & 13 deletions css/frm_admin.css
Original file line number Diff line number Diff line change
Expand Up @@ -2662,7 +2662,7 @@ h2.frm-h2 + .howto {
margin-top: 0;
}

.frm-admin-page-formidableedit #wpbody-content > *:not(.frm-review-notice):not(.frm_previous_install):not(.frm-banner-alert),
.frm-admin-page-formidableedit #wpbody-content > *:not(.frm-review-notice):not(.frm_previous_install):not(.frm-banner-alert):not(.frm_small_device_message_container),
#wpbody-content > .updated,
#wpbody-content > #update-nag,
#wpbody-content > .update-nag,
Expand Down Expand Up @@ -8620,6 +8620,10 @@ span.clear {
display: block;
}

.frm_small_device_message_container {
display: none;
}

/* End hide from IE-mac */

/*-----------------------------------------------------------------
Expand All @@ -8645,6 +8649,16 @@ Responsive Design
.frm_actions_list {
margin-left: 0;
}

#frm_bs_dropdown {
/* There isn't enough room for the title on a screen this size so just hide it. */
display: none;
}

#frm_bs_dropdown .frm_bstooltip {
/* There isn't enough room for the title on a screen this size so just hide it. */
display: none !important;
}
}

@media only screen and (max-width: 960px) {
Expand Down Expand Up @@ -8675,22 +8689,12 @@ Responsive Design
}

@media only screen and (max-width: 850px) {
#frm_bs_dropdown {
/* There isn't enough room for the title on a screen this size so just hide it. */
display: none;
}

.frm-white-body,
.frm_wrap {
--biggest-sidebar: 350px;
--big-sidebar: 350px;
}

#frm_bs_dropdown .frm_bstooltip {
/* There isn't enough room for the title on a screen this size so just hide it. */
display: none !important;
}

.frm_col_one {
clear: none;
}
Expand Down Expand Up @@ -8770,6 +8774,32 @@ Responsive Design
border-radius: var(--small-radius);
border-top-width: 1px;
}

#frm_top_bar ul.frm_form_nav {
padding: 1em 0;
position: fixed;
bottom: 0;
width: 100%;
background: white;
z-index: 99;
transform: translate(-50%, 0);
left: 50%;
}

#frm-publishing {
margin-left: auto;
}
Comment on lines +8791 to +8793
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify navigation bar background contrast

The white background may not provide enough contrast with the content below it. Consider adding a subtle box shadow to visually separate it:

   background: white;
+  box-shadow: 0 -2px 4px rgba(0,0,0,0.1);

Error: No shell scripts or web queries found


🏁 Script executed:

#!/bin/bash
# Retrieve context around the #frm-publishing rule in css/frm_admin.css to check for background and box-shadow properties
rg -n -C 5 "#frm-publishing" css/frm_admin.css

Length of output: 1861


🏁 Script executed:

#!/bin/bash
# Searching for 'background: white' in the repository to determine if it's used in the navigation bar styles.
rg -n "background:\s*white" .

Length of output: 509

Navigation Bar Contrast Improvement: Add a Subtle Box-Shadow

  • The CSS in several files (e.g. in css/frm_admin.css at line 8779) applies a white background to elements in the navigation area.
  • The rule for #frm-publishing (lines 8785–8787) does not include any box-shadow property, which could help visually separate the navigation bar from underlying content.
  • Adding a subtle box-shadow such as
    box-shadow: 0 -2px 4px rgba(0,0,0,0.1);
    would enhance the contrast and improve visual separation.


#frm_top_bar > .frm-full-close:last-child {
margin-left: 0;
}
Comment on lines +8780 to +8797
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add responsive navigation bar styles for mobile view

The changes add styles for a fixed navigation bar at the bottom of the screen on mobile devices. This improves mobile usability but there are a few considerations:

  1. The fixed positioning with transform: translate(-50%, 0) and left: 50% could be simplified. Consider using:
-  transform: translate(-50%, 0);
-  left: 50%;
+  left: 0;
+  right: 0;
  1. The z-index of 99 may need to be higher to ensure the navigation stays above all content. Consider:
-  z-index: 99;
+  z-index: 999;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#frm_top_bar ul.frm_form_nav {
padding: 1em 0;
position: fixed;
bottom: 0;
width: 100%;
background: white;
z-index: 99;
transform: translate(-50%, 0);
left: 50%;
}
#frm-publishing {
margin-left: auto;
}
#frm_top_bar > .frm-full-close:last-child {
margin-left: 0;
}
#frm_top_bar ul.frm_form_nav {
padding: 1em 0;
position: fixed;
bottom: 0;
width: 100%;
background: white;
z-index: 999;
left: 0;
right: 0;
}
#frm-publishing {
margin-left: auto;
}
#frm_top_bar > .frm-full-close:last-child {
margin-left: 0;
}

#frm_bs_dropdown {
display: block;
}

#frm_bs_dropdown .frm_bstooltip {
display: block !important;
}
}

@media only screen and (max-width: 782px) {
Expand Down Expand Up @@ -8890,8 +8920,6 @@ Responsive Design
.frm-admin-page-styles .frm_page_container,
.frm_wrap .frm_page_container {
left: 0 !important;
top: 46px !important;
height: calc( 100vh - 46px ) !important;
}

.frm_single_entry_page.frm_wrap .columns-2 {
Expand Down Expand Up @@ -8927,6 +8955,9 @@ Responsive Design
--biggest-sidebar: 50%;
--big-sidebar: 50%;
}
#frm_top_bar h1 {
min-width: unset; /* Reset the min-width to prevent menu items from stacking vertically */
}
}

@media only screen and (max-width: 600px) {
Expand Down Expand Up @@ -9031,6 +9062,48 @@ Responsive Design
#frm-nav-tabs a {
margin-right: 0;
}
.frm_small_device_message_container {
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
position: fixed;
top: 0;
width: 100%;
height: 100%;
z-index: 100;
background: var(--primary-500);
}

.frm_small_device_message {
text-align: center;
display: flex;
flex-direction: column;
align-items: center;
width: 80%;
padding: 1rem;
background: #fff;
border: solid 1px #fff;
border-radius: var(--medium-radius);
}

.frm_small_device_message::before {
content: '';
position: absolute;
top: 50%;
left: 50%;
transform: translate(-78%, -65%);
width: 900px;
height: 900px;
background-color: #0D7DFC;
border-radius: 50%;
z-index: -1;
opacity: 0.3;
}

.frm_small_device_message > svg {
padding-bottom: 1rem;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Small device message container needs accessibility improvements

The small device message container has some accessibility concerns:

  1. The fixed positioning may trap keyboard focus
  2. Missing ARIA attributes for screen readers
  3. The background overlay lacks proper contrast ratio

Apply these improvements:

.frm_small_device_message_container {
  display: flex;
  flex-direction: column;
  align-items: center;
  justify-content: center;
  position: fixed;
  top: 0;
  width: 100%;
  height: 100%;
  z-index: 100;
  background: var(--primary-500);
+ role="dialog"
+ aria-modal="true"
+ aria-label="Device size warning"
}

.frm_small_device_message {
  text-align: center;
  display: flex;
  flex-direction: column;
  align-items: center;
  width: 80%;
  padding: 1rem;
  background: #fff;
  border: solid 1px #fff;
  border-radius: var(--medium-radius);
+ color: var(--grey-900);
+ box-shadow: 0 2px 4px rgba(0,0,0,0.2);
}

Committable suggestion skipped: line range outside the PR's diff.


/* PRINT */
Expand Down