Navigation superstructure
5/21
threads resolved
This is for the channel only. We need to add grid-area: content to our views throughout the site. I'll do that later in a cleanup MR
It's missing some icons in the navigation sidebar until I talk to Michael about that, but you can review the rest @omadrid
** Testing **
- layout should be responsive. Left column should be thinner on tablet resolutions and it should be replaced by a hamburger menu and sliding menu on mobile resolutions
Request to merge
feat/navigation-superstructure
into
epic/navigation-93
The source branch is 16 commits behind the target branch
Requires 2 more approvals from Deployers and QA. Approved by 
![Olivia Madrid](data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" width="200" height="200"><rect fill-opacity="0"/></svg>)
DeveloperNote to future reviewers: feature flag for this is
navigation-2020Edited by Olivia Madridadded 2 commits
- Developer
review app not working?
- DeveloperResolved by Marcelo Rivera
- Last reply by Marcelo Rivera
changed the description
added 75 commits
- 5263c95a - (wip): Server Side Rendering - work in progress
- 0efe142e - (chore): update with master - closes #2414 (closed)
- ca77f94f - (feat): working mvp of ssr
- 8dfc9b3a - (chore): performance bumps and browser specific delays
- 4399c6bc - (fix): failing spec tests
- 6a3dac72 - (fix): plotly fix, preparation for full server bundle and xsrf token re-introduced
- 14ab8c6e - (chore): ci configs
- 46f380ca - (fix): do not cache mobile incorrectly
- cfe0cb4a - (chore): some minor fixes for mobile improvements
- 361ef20c - (fix): do not show permissions if logged out
- 7cb62c62 - (chore): introduce build optimizer
- e9eee3a7 - (chore): preload material icons to improve first paint
- cd9272cd - (feat): meta service for SEO
- 2a206794 - (refactor): SEO from backend to frontend
- 09374b66 - (refactor): remove all uses of window.Minds
- c53c0aaa - (refactor): storage to cookies where applicable
- 4fda5f24 - (chore): allow for epic branches to push docker containers to aws
- 17b9abb5 - (fix): do not return null for nsfw reasons
- 7652c43d - (fix): remind asset links
- ff284a94 - (chore): do not stretch checkboxes
- 8d424529 - (fix): css should be loaded with correct asset path
- befad6c1 - (chore): remove icon preload
- eaf8a773 - (chore): cookie notice for all
- fc1185d7 - (chore): reduce plotly bundle
- af02de4b - (fix): nsfw cookie fix and notices.json credential error
- 3c5dae2d - (fix): wallet not loading when loaded server side
- 2118f305 - (fix): groups not loading server side
- b5b38399 - (fix): do not applu 100% width to checkboxes on homepage
- 26795138 - (fix): bad url for avatars
- b73ee1b0 - (chore): fix seo for blogs
- 98fd4fbe - (chore): temporary favicons
- eeb4b9e1 - (fix): groups create page not loading
- 9c600c2c - Merge branch 'master' of https://gitlab.com/Minds/front into epic/SSR
- 1363cae7 - (fix): canary assets path
- 96c066b0 - (fix): sockets issue
- 03a80ca3 - (fix): wrong path for canary bg
- a0ab3d48 - (fix): try not to render css for checkboxes
- b876854f - (fix): only show more groups if we have groups
- c3b33785 - Merge branch 'master' of https://gitlab.com/Minds/front into epic/SSR
- 5a27ec5f - (feat): checkboxes on homepage
- a4d3e5e9 - (chore): update local build and serve scripts
- 95c2bfe7 - (fix): pro sites should wait for configs to be resolved
- 3a90b3dc - (fix): date dropdown should be in common posdt master merge
- adb6015c - (fix): reset router if pro
- f15627e1 - (fix): pro fixes
- 176a3c8f - (chore): move sentry to diagnostics service
- fc6a8b2e - (chore): sentry capture exception for pro
- 386ca4f8 - (fix): only render css bindings if in browser
- 2b3b74b3 - (chore): add Object type top platformId
- 75639885 - (fix): wrap session storage in try catch
- 7c5d306b - (fix): og tags incorrect
- da3ed520 - Merge branch 'master' of https://gitlab.com/Minds/front into epic/SSR
- 13422e7a - [Sprint/HipsterHedgehog] (feat): ability to copy and paste images into posts and comments
- 2cb1c987 - Merge branch 'feat/copy-and-paste-images-in-posts' into 'master'
- 4f327c86 - Plyr play error
- 184c0f46 - Merge branch 'fix/autoplay-bug-2377' into 'master'
- 2809d08f - (chore): adds in production branch to be able to deploy to production
- 5fede3a6 - Merge branch 'master' of https://gitlab.com/Minds/front into epic/SSR
- 145a730f - (chore): some small ci changes to test production pipeline works with SSR
- 225dc6f3 - (chore): allow for master to hit preprod
- 6b1b3271 - Modal pager fixes
- c6474453 - Merge branch 'fix/modal-pager-sync' into 'production'
- ff63f69d - Merge branch 'fix/modal-pager-sync' into 'production'
- fae30718 - Search period defaults and sort bar tweaks
- 410e752c - Merge branch 'chore/default-search-period' into 'master'
- 2261e73f - (chore): remove review dependency
- 5ffd58f5 - Merge branch 'production' of https://gitlab.com/Minds/front
- 44667f0d - (chore): bring sandbox helm chart branch to master
- fd261feb - (fix): remind preview cdn url - fixes #2531 (closed)
- 6fc73b99 - New Channel Registration, Signup button remains disabled
- 759cb8aa - Merge branch 'fix/register-issue-2527' into 'master'
- 6d761b80 - Merge remote-tracking branch 'upstream/master' into feat/navigation-superstructure
- 3890c40a - (fix): remove references to window.Minds
- 707b5089 - (fix): remove unused imports
- 380ea055 - (fix): only set .has-markers-sidebar if the new nav isn't enabled
Toggle commit listadded 2 commits
changed the description
resolved all threads
approved this merge request
44 align-items: center; 45 min-width: 0; 46 } 47 48 @media screen and(min-width: $m-grid-max-mobile) { 49 .m-v3topbar__leftColumn, 50 .m-v3topbar__middleColumn { 51 @include m-theme() { 52 border-right: 1px solid themed($m-grey-400); 53 } 54 } 55 } 56 } 57 } 58 59 .m-v3-topbar__Top { - Owner
We need consistency here.
- Resolved by Marcelo Rivera
- Resolved by Marcelo Rivera
- Resolved by Marcelo Rivera
1 1 <ng-container *ngIf="ready"> 2 2 <ng-container *ngIf="!isProDomain"> 3 <m-v2-topbar> 4 <ng-container search> 5 <m-search--bar [defaultSizes]="false"></m-search--bar> 6 </ng-container> 3 <ng-container *ngIf="useNewNavigation; else v2Topbar"> - Owner
These labels need to be more contextual ie. not
useNewNavigationandlegacyTopbar. Why has legacyTopbar returned as well?
30 <m-notifications--topbar-toggle></m-notifications--topbar-toggle> 31 <m-wallet--topbar-toggle></m-wallet--topbar-toggle> 32 </ng-container> 33 </m-topbar> 34 </ng-template> 35 36 <m-sidebar--markers 37 [class.has-v2-navbar]="featuresService.has('top-feeds')" 38 ></m-sidebar--markers> 39 </ng-template> 18 40 </ng-container> 19 41 20 42 <m-body 21 [class.has-markers-sidebar]="session.isLoggedIn() && !isProDomain" 43 [class.has-markers-sidebar]=" 44 session.isLoggedIn() && - Owner
this is getting too big and should be in the component.
- Resolved by Marcelo Rivera
457 463 deps: [Title, Meta, SiteService, Location, ConfigsService], 458 464 }, 459 465 MediaProxyService, 460 V2TopbarService, 466 SidebarNavigationService, 467 { 468 provide: V2TopbarService, 469 useFactory: V2TopbarService._, 470 }, 471 { 472 provide: SidebarNavigationService, 473 useFactory: SidebarNavigationService._, - Owner
Do we need a factory? The class has an
@Injectableon it
12 text-decoration: none; 13 font-family: 'Roboto', Helvetica, sans-serif; 14 padding: 8px 0; 15 @include m-theme() { 16 color: themed($m-grey-300); 4 .m-sidebarNavigation__overlay { 5 position: fixed; 6 top: 0; 7 bottom: 0; 8 left: 0; 9 right: 0; 10 z-index: -1; 11 background-color: transparent; 12 transition: background-color 0.5s cubic-bezier(0.075, 0.82, 0.165, 1); 13 14 &.m-sidebarNavigation--opened { - Owner
modified on
__overlay?
101 ul { 102 list-style: none; 103 padding: 0; 104 } 105 106 h5 { 107 font-size: 11px; 108 line-height: 30px; 109 letter-spacing: 1.83px; 110 text-transform: uppercase; 111 @include m-theme() { 112 color: rgba(themed($m-black), 0.3); 113 } 114 } 115 116 m-group--sidebar-markers { - Owner
group logic outside of groups?
9 22 }) 10 export class SidebarNavigationComponent { 23 export class SidebarNavigationComponent implements OnInit { 24 readonly cdnAssetsUrl: string; 25 26 @ViewChild(DynamicHostDirective, { static: true }) host: DynamicHostDirective; 27 11 28 user; 12 items; 13 29 14 constructor(public navigation: NavigationService, public session: Session) { 15 this.items = navigation.getItems('sidebar'); 30 componentRef; 31 componentInstance: GroupsSidebarMarkersComponent; 32 33 isDesktop: boolean = true; - Owner
Two competing booleans.
37 isOpened: boolean = false; 38 39 constructor( 40 public navigation: NavigationService, 41 public session: Session, 42 private service: SidebarNavigationService, 43 protected configs: ConfigsService, 44 private _componentFactoryResolver: ComponentFactoryResolver, 45 @Inject(PLATFORM_ID) private platformId: Object 46 ) { 47 this.cdnAssetsUrl = this.configs.get('cdn_assets_url'); 48 this.service.setContainer(this); 16 49 this.getUser(); 50 51 if (isPlatformBrowser(this.platformId)) { 52 this.onResize(); - Owner
This should be removed from the constructor
1 import { Injectable } from '@angular/core'; 2 import { SidebarNavigationComponent } from './navigation.component'; 3 4 @Injectable() 5 export class SidebarNavigationService { 6 container: SidebarNavigationComponent; 7 8 static _() { 9 return new SidebarNavigationService(); 10 } 11 12 setContainer(container: SidebarNavigationComponent) { 13 this.container = container; 14 } 15 16 toggle() { - Owner
return types
1 <ng-template #searchBar> 2 <div class="m-v3-topbar__SearchBox"> 3 <ng-content select="[search]"></ng-content> 4 </div> 5 </ng-template> 6 7 <div class="m-v3-topbar__Top"> 8 <div class="m-grid"> 9 <div class="m-v3topbar__leftColumn"> 10 <nav class="m-v3-topbar__Nav"> - Owner
157 > 158 Settings 159 </m-tooltip> 160 161 <span 162 class="m-sidebar--navigation--text" 163 *ngIf="isDesktop || isMobile" 164 i18n 165 > 166 Settings 167 </span> 168 </a> 169 </li> 170 </ul> 171 </nav> 172 <h5 i18n *ngIf="isDesktop || isMobile"> - Owner
ideally this would be in its own isolated component.
27 27 <span 28 28 class="mdl-color-text--blue-grey-300" 29 29 i18n="@@MINDS__CHANNELS__ERROR_CHECK_USERNAME" 30 >Please check the username</span 31 30 > 31 Please check the username 32 </span> 32 33 </div> 33 34 34 <header [hidden]="!isLocked"></header> 35 <ng-container *mIfFeature="'navigation-2020'; else oldChannel"> 36 <div class="m-channel__middleColumn"> - Owner
this is actually left column not middle?
27 27 <span 28 28 class="mdl-color-text--blue-grey-300" 29 29 i18n="@@MINDS__CHANNELS__ERROR_CHECK_USERNAME" 30 >Please check the username</span 31 30 > 31 Please check the username 32 </span> 32 33 </div> 33 34 34 <header [hidden]="!isLocked"></header> 35 <ng-container *mIfFeature="'navigation-2020'; else oldChannel"> - Owner
Do not use old or legacy
1 <div class="m-channelSidebar__buttons"> 2 <button class="m-channelSidebar__message" i18n> - Owner
css should focus on this being a button
1 <div class="m-channelSidebar__buttons"> 2 <button class="m-channelSidebar__message" i18n> 3 <i class="material-icons">sentiment_satisfied_alt</i> 4 <span>Message</span> 5 </button> 6 7 <button class="m-channelSidebar__pay" i18n> 8 <i class="material-icons">sentiment_satisfied_alt</i> 9 <span>Pay</span> 10 </button> 11 </div> 12 13 <div class="m-channelSidebar__stats"> - Owner
Recommend splitting this out into smaller components. This is getting a very large component.
50 </a> 51 <a> 52 <span class="m-channelSidebarStats__title" i18n> 53 Views 54 </span> 55 <span class="m-channelSidebarStats__count">{{ 56 user.impressions | abbr 57 }}</span> 58 </a> 59 </div> 60 61 <div class="m-channelSidebar__bioFields"> 62 <!-- [ngClass]="{'m-channelSidebar__bioFields--flex': !editing}"--> 63 <!-- City / Location --> 64 <div 65 class="m-channelSidebar__bioField m-channelSidebar__city" - Owner
__cityis a modifier?
added scoped label
