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

Toggle Menu not working in Angular #398

Closed
puravupadhyay opened this issue Feb 20, 2019 · 13 comments
Closed

Toggle Menu not working in Angular #398

puravupadhyay opened this issue Feb 20, 2019 · 13 comments

Comments

@puravupadhyay
Copy link

Bug Report

What is the issue?

I'm trying to use this library in an Angular project. I installed nhsuk-frontend via npm fine.
I managed to import the styles and can see all the component rendering but I'm having problems with the javascript files. I imported all the modules like suggested but not able to make the Menu Toggle work. Can someone please help?

What steps are required to reproduce the issue?

  1. Create an Angular app, install nhsuk-frontend using NPM.
  2. Include the NHS nav header in the app.component.html
  3. Import the javascript modules in the app.component.ts
  4. Run the app, keep reducing the screen size until the Menu button appears. Click on it and nothing happens.

What was the environment where this issue occurred?

  • Device: PC / MAC
  • Operating System: Windows 10, macOS
  • Browser: All
  • Browser version: -
  • NHS.UK frontend package version: 1.0.0
  • Node version: 11.10.0
  • npm version: 6.7.0

Is there anything else you think would be useful in recreating the issue?

This is the sample code of app.component.ts file:

import nhsuk_feedbackBanner from 'node_modules/nhsuk-frontend/packages/components/feedback-banner/feedback-banner';
import nhsuk_header from 'node_modules/nhsuk-frontend/packages/components/header/header';
import nhsuk_skipLink from 'node_modules/nhsuk-frontend/packages/components/skip-link/skip-link';
import autocomplete from 'node_modules/nhsuk-frontend/packages/components/header/autocomplete';

@Component({
  selector: 'app-root',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.scss']
})
export class AppComponent {

  title = 'NHSFrontEnd';


  ngOnInit() {
    nhsuk_feedbackBanner(3000);
    nhsuk_header();
    nhsuk_skipLink();
    autocomplete();
  }

}

nhs header

@chrimesdev
Copy link
Member

Are there any errors in the browser console?

@puravupadhyay
Copy link
Author

No errors in the console

console

@chrimesdev
Copy link
Member

Haven’t used Angular in a while so I will ask around to see if anyone else has tried using it with Angular and I will see if I can replicate it.

@puravupadhyay
Copy link
Author

Ok. Thanks Adam!

@chrimesdev chrimesdev added the help wanted Extra attention is needed label Feb 20, 2019
@chrimesdev
Copy link
Member

If you are keen to get it working (whilst we investigate further) you could use the compiled nhsuk.min.js file from the node_modules folder rather than ES6 modules.

You can find instructions here: https://github.com/nhsuk/nhsuk-frontend/blob/master/docs/installation/installing-with-npm.md#option-1-include-compiled-javascript

@mikemonteith
Copy link
Contributor

My guess would be that the header JS is trying to select the menu button before it's been rendered to the page.

Here, we are doing the select outside of the initialisation function, when it should be inside.
https://github.com/nhsuk/nhsuk-frontend/blob/master/packages/components/header/header.js#L53

@puravupadhyay
Copy link
Author

puravupadhyay commented Feb 21, 2019

Just tried a bit of debugging, stepping into Header.js.

header js debugging

Also, I've pushed this project up: https://github.com/puravupadhyay/nhsuk-frontend-angular

Cheers

chrimesdev added a commit that referenced this issue Feb 22, 2019
Globally scoped variables were causing issues with JavaScript frameworks
such as Angular, so they have been moved to be locally scoped within
functions. This could be improved as we have to duplicate variables.
chrimesdev added a commit that referenced this issue Feb 22, 2019
The wrap-search element was not being found as the page load event
listener was coming after the search for the element which caused issues
in JavaScript frameworks such as Angular.
@chrimesdev
Copy link
Member

Hi @puravupadhyay

Just to let you know, I've opened a draft pull request with a potential fix for this #402 I'm just testing it out with your Angular application (and other applications) now. Also looking at potential ways we could improve our JavaScript, so if you have any advice that would be greatly appreciated.

Thanks for raising this with us, hopefully we should get it fixed soon.

chrimesdev added a commit that referenced this issue Feb 22, 2019
The wrap-search element was not being found as the page load event
listener was coming after the search for the element which caused issues
in JavaScript frameworks such as Angular.
@puravupadhyay
Copy link
Author

Thanks @AdamChrimes for that. I tested the changed files and they're working.

Just a thought - could use the namespaces in JavaScript - not 100% confident that it will work. Will have to play-around with it a bit, I'll try and set the project up my end. I guess this is the best place to look for guidance on that: https://github.com/nhsuk/nhsuk-frontend/tree/master/docs/contributing

Cheers

chrimesdev added a commit that referenced this issue Feb 28, 2019
Globally scoped variables were causing issues with JavaScript frameworks
such as Angular, so they have been moved to be locally scoped within
functions. This could be improved as we have to duplicate variables.
chrimesdev added a commit that referenced this issue Feb 28, 2019
The wrap-search element was not being found as the page load event
listener was coming after the search for the element which caused issues
in JavaScript frameworks such as Angular.
@chrimesdev
Copy link
Member

chrimesdev commented Mar 11, 2019

Hi @puravupadhyay, we've just released version 2.0.0 of the Frontend library. With this release is the fix we mentioned above for the JavaScript variables.

I've just tested it with your Angular repo with npm install [email protected] and it seems to be working. Shouldn't need to update any files just the dependency.

We're still looking at improving our JavaScript overall in the issue #408 but this should fix it in the meantime.

Thanks again for raising this issue, it will be good to keep in touch. Do you use Slack? We have a public Slack instance which may easier to share progress https://nhs-service-manual.slack.com/

@chrimesdev chrimesdev added testing and removed help wanted Extra attention is needed labels Mar 11, 2019
@puravupadhyay
Copy link
Author

@AdamChrimes The new version works! Would be great to use it in future projects.

Sure, how do I join https://nhs-service-manual.slack.com/ ? I don't think I have a Slack account.

@chrimesdev
Copy link
Member

Great thanks @puravupadhyay

You will need to create an account (its free) you can sign up with your @nhs.net email address here: https://nhs-service-manual.slack.com/signup

or if you want to use another email account provider you can use this link

You can use Slack in the browser or you can download the application. Not to worry if you're not wanting to join Slack - just a useful tool for instant message and sharing quickly.

@chrimesdev
Copy link
Member

Closing as this is fixed in version 2.0.0.

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

No branches or pull requests

3 participants