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

dev/core#1987: Fix Drupal Base 'isFrontEndPage' function to consider Drupal public page for FE theme #18397

Merged

Conversation

swastikpareek
Copy link
Contributor

@swastikpareek swastikpareek commented Sep 7, 2020

Overview

Drupal webform pages may have CiviCRM fields which internally initialize CiviCRM assets and was loading CiviCRM current theme assets. This is because CiviCRM doesn't have the logic for checking Drupal webform pages, whether they are FE pages or backend and since the logic is incomplete it loads the backend theme which because of this logic.

Extract

The whole idea is to tell Civicrm to treat non civicrm pages (Drupal public pages) which loads civicrm code to treat them as the front end page and apply front end theme. Currently it doesn't it treats these pages as backend pages and apply the civicrm backend theme on them.

Before

The CiviCRM was loading backend theme for the Drupal public web form pages which loads CiviCRM components.
Webform page which has civicrm enabled
Screenshot 2020-09-24 at 3 45 53 PM

After

The CiviCRM was loading the front theme for the Drupal public web form pages which loads CiviCRM components.
Webform page which has civicrm enabled
Screenshot 2020-09-24 at 3 48 03 PM

Technical Details

  • isFrontEndPage is updated so that it treats Drupal public web form pages as CiviCRM public pages and load front end theme there.

Note

The before and after screenshots are added after adding a var_dump in getActiveThemeKey function in Themes.php.
Screenshot 2020-09-24 at 3 50 01 PM

@civibot
Copy link

civibot bot commented Sep 7, 2020

(Standard links)

@eileenmcnaughton
Copy link
Contributor

@agileware-justin is this something you can get one of your team to test/ review?

@agileware-justin
Copy link
Contributor

@eileenmcnaughton as long as you want this to be tested on Drupal 7?

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 that seems fine to me - do you think d8 is an issue here?

@demeritcowboy
Copy link
Contributor

For drupal 8 I did a little clicking around. I didn't test webform, just that existing pages seemed to function the same, e.g. contribution pages and profiles get the front end theme, regular drupal nodes look normal, and civi pages look like civi pages.

@agileware-justin
Copy link
Contributor

I've tested this patch and can't see any difference, sorry.

I think if you want someone to validate what this PR is doing with confidence, maybe include detailed steps to reproduce and for bonus points, have before / after screenshots.

@swastikpareek
Copy link
Contributor Author

I've tested this patch and can't see any difference, sorry.

I think if you want someone to validate what this PR is doing with confidence, maybe include detailed steps to reproduce and for bonus points, have before / after screenshots.

@agileware-justin : I have added Before and After screenshots to the PR description and have also updated PR and added the extract of the problem and added a small note to compliment the details.

Let me know if you need further information to understand the issue better.

Thanks

@seamuslee001
Copy link
Contributor

This makes sense to me, I think if there is no CRM Menu route then lets assume it is front end, merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants