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.

Sign up for GitHub

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

Jump to bottom

Prevent unlogged rendering #451

Merged
merged 6 commits into from
Apr 5, 2019
Merged

Prevent unlogged rendering #451

merged 6 commits into from
Apr 5, 2019

Conversation

parostatkiem-zz
Copy link
Contributor

Description
✔️ Tested with all combinations of auth.use, auth.disableAutoLogin and login state.

Changes proposed in this pull request:

  • Add isHidden attribute to Authorization component which makes it not render anything, just fire onCreate hook (and proceed autologin)
  • Render the whole TopNav only if needed. Otherwise, "display" hidden Authorization.

Related issue(s)

@parostatkiem-zz parostatkiem-zz added enhancement New feature or request area/luigi labels Apr 1, 2019
@maxmarkus maxmarkus self-assigned this Apr 1, 2019
maxmarkus
maxmarkus approved these changes Apr 2, 2019
View reviewed changes
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

jesusreal
jesusreal reviewed Apr 3, 2019
View reviewed changes
Copy link
Contributor

@jesusreal jesusreal left a comment

Choose a reason for hiding this comment

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

I see we are still rendering the anonymous content. If possible, could we just render nothing and redirect to login page?

jesusreal
jesusreal suggested changes Apr 3, 2019
View reviewed changes
Copy link
Contributor

@jesusreal jesusreal left a comment

Choose a reason for hiding this comment

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

Please see my comments

core/src/navigation/TopNav.html Outdated Show resolved Hide resolved
core/src/Authorization.html Show resolved Hide resolved
core/src/navigation/TopNav.html Outdated Show resolved Hide resolved
@parostatkiem-zz parostatkiem-zz merged commit 9ec3b9b into SAP:master Apr 5, 2019
@parostatkiem-zz parostatkiem-zz deleted the Prevent-unlogged-rendering branch April 5, 2019 07:28
hardl referenced this pull request in hardl/luigi Apr 5, 2019
@hardl
m (#176)
17bf8be 
* Deactivate Typescript declaration file for Luigi Client (#466)

* Prevent unescaped characters in the Alert component (#461)

* ios scroll (#469)

* Prevent unlogged rendering (#451)

* Add ‘isHidden’ prop to Authorization component

* Render TopNav only if needed
stanleychh pushed a commit to stanleychh/luigi that referenced this pull request Dec 30, 2021
@parostatkiem
Prevent unlogged rendering (SAP#451)
ab054a8 
* Add ‘isHidden’ prop to Authorization component

* Render TopNav only if needed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxmarkus maxmarkus maxmarkus approved these changes

@jesusreal jesusreal jesusreal approved these changes

@antiheld antiheld Awaiting requested review from antiheld

@dariadomagala-sap dariadomagala-sap Awaiting requested review from dariadomagala-sap

@hardl hardl Awaiting requested review from hardl hardl is a code owner

@JohannesDoberer JohannesDoberer Awaiting requested review from JohannesDoberer JohannesDoberer is a code owner

@kwiatekus kwiatekus Awaiting requested review from kwiatekus

@marynaKhromova marynaKhromova Awaiting requested review from marynaKhromova

@pekura pekura Awaiting requested review from pekura

@y-kkamil y-kkamil Awaiting requested review from y-kkamil

Assignees

@maxmarkus maxmarkus

@jesusreal jesusreal

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@parostatkiem-zz @maxmarkus @jesusreal