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

Fix css issues in core and angular example #140

Merged

Conversation

jesusreal
Copy link
Contributor

Description

Changes proposed in this pull request:

  • Make logout button visible over iframe container (core)
  • Fix logout button styling and functionality
  • Fix styling on external page (angular example)
  • Fix styling on logout page (angular example)

Related issue(s)

Copy link
Contributor

@parostatkiem-zz parostatkiem-zz left a comment

Choose a reason for hiding this comment

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

All issues were explained via call and nothing has to be changed

Copy link
Contributor

@maxmarkus maxmarkus left a comment

Choose a reason for hiding this comment

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

Just a question above

@@ -9,6 +9,7 @@
<meta name="description" content="">
<meta name="keywords" content="logout">

<link rel='stylesheet' href='../node_modules/fundamental-ui/dist/fundamental-ui.min.css'>
Copy link
Contributor

Choose a reason for hiding this comment

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

I fear this approach will not work once the user has don a npm run build and deploys the dist folder, as it will miss the fundamental-ui then?

Copy link
Contributor

@maxmarkus maxmarkus left a comment

Choose a reason for hiding this comment

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

npm run startWebpack does not add fundamental styles

@@ -397,6 +397,27 @@ module.exports = {
from: {
glob: 'luigi-client.js'
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

The css file is copied, but not injected into index.html
A quick tryout of webpack.config.js entry.styles: ['./src/styles.css', './node_modules/fundamental-ui/dist/fundamental-ui.min.css'] did not work in my case, needs investigation how to reference the css properly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively manually put it into sampleapp.html and remove the injection reference in angular.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fundamental-ui.min.css injected via webpack.config.js. Please check again.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good job!

Copy link
Contributor

@maxmarkus maxmarkus left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -397,6 +397,27 @@ module.exports = {
from: {
glob: 'luigi-client.js'
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Good job!

@jesusreal jesusreal merged commit 71be5a0 into SAP:master Oct 18, 2018
@jesusreal jesusreal deleted the fix-css-issues-in-core-and-angular-example branch March 13, 2019 10:30
stanleychh pushed a commit to stanleychh/luigi that referenced this pull request Dec 30, 2021
* Load fundamental-ui correctly in external views on angular angular example

* add missing fundamental-ui import on authorization html

* Restructure markup so that logout button is displayed over iframe

* Enable external views in angular example to use fundamental ui after building app

* Load fundamental-ui only once in example app

* In webpack config, provide app with fundamental-ui.min.css and fix path for fundamental ui folder for external views

* Bring back loading indicator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants