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

ion-navbar take not into account with $navbar-ios-height #12811

Closed
seed880505 opened this issue Sep 4, 2017 · 6 comments
Closed

ion-navbar take not into account with $navbar-ios-height #12811

seed880505 opened this issue Sep 4, 2017 · 6 comments
Assignees
Labels
ionitron: v3 moves the issue to the ionic-v3 repository

Comments

@seed880505
Copy link

Resources:
Before submitting an issue, please consult our troubleshooting guide (http://ionicframework.com/docs/troubleshooting/) and developer resources (http://ionicframework.com/docs/developer-resources/)

Ionic version: (check one with "x")
[ ] 1.x (For Ionic 1.x issues, please use https://github.com/ionic-team/ionic-v1)
[ ] 2.x
[x] 3.x

I'm submitting a ... (check one with "x")
[x] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or http://ionicworldwide.herokuapp.com/

Current behavior:

Define scss variable $navbar-ios-height, but navbar only take into accout with $toolbar-ios-height.

Expected behavior:

ion-navbar should take into account $navbar-ios-height, instead of $toolbar-ios-height.

Steps to reproduce:

Related code:

$navbar-md-height: 80px; // ignored
$toolbar-md-height: 40px; // true height

Other information:

Ionic info: (run ionic info from a terminal/cmd prompt and paste output below):

cli packages:
    @ionic/cli-utils  : 1.9.2
    ionic (Ionic CLI) : 3.9.2

global packages:
    Cordova CLI : 7.0.1

local packages:
    @ionic/app-scripts : 2.1.4
    Cordova Platforms  : none
    Ionic Framework    : ionic-angular 3.6.0

System:
    Android SDK Tools : 26.0.2
    Node              : v7.10.1
    npm               : 4.2.0
    OS                : Windows 7
@kensodemann
Copy link
Member

Hello! Thank you for opening an issue with us!

Looking at the code, we set $navbar-ios-height as such in toolbar.ios.scss, but then never use it, anywhere:

/// @prop - Height of the navigation bar
$navbar-ios-height:                         $toolbar-ios-height !default;

Similar for $navbar-md-height

We will look into this.

Thank you for using Ionic

@kensodemann kensodemann added the css label Sep 4, 2017
@kensodemann
Copy link
Member

After internal discussions, we believe that $navbar-*-height were left over from a refactoring that was done in the past which combined a lot of styling between ion-toolbar and ion-navbar so for now you should set the $toolbar-ios-height and $toolbar-md-height as needed.

We will look at refactoring this again in v4 to allow for better styling of the two components.

Associated notes from @brandyscarney -

there are some duplicated styles between .toolbar and ion-navbar.toolbar. I’d say we need to add a navbar class for the toolbar inside of a navbar and let them style it completely separate.

this is something we’ll have to look over for v4 anyways since ion-toolbar and ion-navbar currently share the same template but are two different components

@kensodemann kensodemann added this to the 4.x milestone Sep 5, 2017
@cabaird
Copy link

cabaird commented Oct 2, 2017

@kensodemann ,

Do you have a solution for changing just the navbar height when you have a subheader? The solution you posted would adjust both the navbar and the subheader heights to the variable.

Thank you,

Christian

@cabaird
Copy link

cabaird commented Oct 2, 2017

Should have tried before posting (I have an aversion to changing css, so was hoping for a way to do this more cleanly). However, adding the following to the app scss seems to work fine, and if so, is clean enough for me:

ion-navbar {
  height: 66px;
}

I was worried that the content would not line up as expected when adjusting the css directly and not using a variable, but it seems to be working.

@Ionitron Ionitron added the ionitron: v3 moves the issue to the ionic-v3 repository label Nov 28, 2018
@ionitron-bot
Copy link

ionitron-bot bot commented Nov 28, 2018

This issue has been automatically identified as an Ionic 3 issue. We recently moved Ionic 3 to its own repository. I am moving this issue to the repository for Ionic 3. Please track this issue over there.

If I've made a mistake, and if this issue is still relevant to Ionic 4, please let the Ionic Framework team know!

Thank you for using Ionic!

@ionitron-bot
Copy link

ionitron-bot bot commented Nov 28, 2018

Issue moved to: ionic-team/ionic-v3#243

@ionitron-bot ionitron-bot bot closed this as completed Nov 28, 2018
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Nov 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ionitron: v3 moves the issue to the ionic-v3 repository
Projects
None yet
Development

No branches or pull requests

5 participants