Skip to content
This repository has been archived by the owner on Mar 25, 2023. It is now read-only.

feat: keep sidenav visibility state on the server for every user #1194

Merged
merged 2 commits into from
Aug 15, 2018

Conversation

tamazlykar
Copy link
Collaborator

@tamazlykar tamazlykar commented Aug 10, 2018

Closes #944

We omit the result of setting the tag value representing sidenav status on the server.
This is required so that the UI reacts instantly and does not wait until an answer comes from the server.
Downsides: if the tag is not set, the user selected state will not be saved (this can happen very rarely)

public toggleDrawer(): void {
this.layoutService.toggleDrawer();
public closeSidenav(): void {
this.store.dispatch(new layoutActions.CloseSidenav);
Copy link
Contributor

Choose a reason for hiding this comment

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

please, add brackets '()'

@@ -16,10 +15,10 @@ import * as serviceOfferingActions from '../reducers/service-offerings/redux/ser
})
export class HomeComponent extends WithUnsubscribe() implements OnInit {
public disableSecurityGroups = false;
public showSidenav$ = this.store.select(layoutSelectors.getIsShowSidenav);
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you use this language?
"is show sidenav" why not to use" "is sidenav visible"

Copy link
Contributor

@zolotyx zolotyx left a comment

Choose a reason for hiding this comment

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

Looks good. please add the changes to the first comment

@tamazlykar
Copy link
Collaborator Author

Changes added

@tamazlykar tamazlykar merged commit 02ae499 into bwsw:master Aug 15, 2018
@tamazlykar tamazlykar deleted the 994-sidenav-position branch August 15, 2018 08:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants