Integrate responsive preview with Navigation top bar

Created on 22 November 2024, 3 months ago

Problem/Motivation

It will be good to have a desktop/mobile switcher available in the navigation top bar provided by Drupal Core.

Proposed resolution

Drupal core has introduced the Navigation module, which includes a top bar feature. This top bar enables other modules to define items using the @TopBarItem plugin. As part of this issue, a top bar tool will be created to preview the desktop and mobile versions of the current page.

Remaining tasks

This implementation will depend on https://www.drupal.org/project/drupal/issues/3484564 📌 Define the 3 areas the Top Bar will provide Active

User interface changes

The new top bar tool will look like following

API changes

Data model changes

Feature request
Status

Active

Version

2.2

Component

Code

Created by

🇮🇳India kunalkursija Mumbai

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @kunalkursija
  • 🇮🇳India anjali rathod India

    I am facing issue setting up the responsive preview module. Attaching the screen recording video for reference.

  • 🇮🇳India roshni upadhyay

    Hi @lauriii,

    I encountered an issue while working with the responsive_preview module. The error was as follows:

    Error: Call to undefined function drupal_common_theme() in responsive_preview_theme() (line 40 of modules/contrib/responsive_preview/responsive_preview.module)

    I resolved it by modifying the code in the responsive_preview_theme() function. Could you please confirm whether this should be addressed as a separate issue, or can we include the fix under the scope of the current issue?

    Additionally, I need clarification regarding the default dimensions for the device icons. What dimensions should we use for both devices? Please provide the recommended width and height values to standardize the implementation.

  • 🇮🇳India roshni upadhyay

    I would like to clarify that I am still facing the issue. In my previous comment, I mentioned that it was working, but that was due to a version mismatch. The issue persists as follows:

    When the Responsive Preview block is placed, the buttons are displayed but do not function as expected.
    If the Navigation module is uninstalled, the Responsive Preview list displays correctly in the toolbar and works perfectly.

  • 🇮🇳India roshni upadhyay

    The Responsive Preview block was not functioning as expected due to the missing #block-responsivepreviewcontrols ID when the block was placed. After adding the required ID, the block started working as intended. With this issue resolved, I am now unblocked and able to continue working on it.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 1503s
    #394259
  • Pipeline finished with Failed
    about 1 month ago
    Total: 1768s
    #394350
  • Pipeline finished with Failed
    about 1 month ago
    Total: 1491s
    #394490
  • 🇪🇸Spain plopesc Valladolid

    @roshni upadhyay Thank you for your work on this one.

    After reading the code, I have some ideas to make it a bit more generic:

    * We could create the navigation top bar as a submodule, or even a new contrib module that depends on this one and navigation. Let's work on a MR for this one for now.
    * Add a 3rd party setting to the config entity as part of this submodule to "Show in Top Bar" the responsive_preview entity
    * Instead of hardcoding the types in ResponsiveIcons class, load the items whose 3rd party setting is defined to be shown in the top bar
    * Think in a way of how to display those options different from "mobile" & "desktop". That could be part of a follow-up issue

  • 🇪🇸Spain plopesc Valladolid

    Another approach could be to make it fully hardcoded, and do not make use of the config entities, and use the original module as an API to provide the logic to switch between devices, but not crating new entities, nor using them.

    This 2nd approach could be simpler to implement at this point, but we will probably want something more generic in the long term.

  • 🇫🇮Finland lauriii Finland

    Discussed with @plopesc and @roshni upadhyay to align on the approach. We agreed that we will continue with the approach proposed in #11. What we will do is:

    1. Create a new submodule specifically for Drupal CMS (and Experience Builder)
    2. For now, the submodule will not be using the Device config entity but we will hard code those values in the Drupal\navigation\TopBarItemBase implementation which will allow us to provide a good experience consistently
    3. We will avoid making changes to the main module for now so that we can ship the submodule in a separate project if necessary
    4. In future, we might integrate the device settings with Experience Builder to automatically define them based on what has been configured there.
  • Pipeline finished with Failed
    about 1 month ago
    Total: 1827s
    #398051
  • Pipeline finished with Failed
    about 1 month ago
    Total: 1943s
    #398445
  • 🇪🇸Spain plopesc Valladolid

    Changes look great and goes in the right direction.

    Added some comments in the MR that might need to be addressed.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 1512s
    #400825
  • Pipeline finished with Failed
    about 1 month ago
    Total: 1441s
    #401301
  • Assigned to roshni upadhyay
  • Status changed to Needs review about 1 month ago
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 91s
    #402496
  • 🇪🇸Spain plopesc Valladolid

    Made a couple of minor adjustments, but overall, I think this is ready for final review.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 1181s
    #402497
  • Pipeline finished with Failed
    about 1 month ago
    Total: 1586s
    #402512
  • 🇫🇮Finland lauriii Finland

    This looks pretty good based on the functionality and code has been reviewed by @plopesc.

    I think an important follow-up to this would be to render the responsive preview as an anonymous user. That might lead into some complications so it's better to leave that to another issue.

  • 🇪🇸Spain plopesc Valladolid

    Created follow-up 📌 Preview displays include Navigation Toolbars Active

  • First commit to issue fork.
  • 🇮🇳India rajeshreeputra Pune

    Rebased, will merge this and wait for the follow-up 📌 Preview displays include Navigation Toolbars Active .

Production build 0.71.5 2024