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] - Header Menu Size #1151

Merged
merged 5 commits into from
May 2, 2020
Merged

[fix] - Header Menu Size #1151

merged 5 commits into from
May 2, 2020

Conversation

ibelar
Copy link
Contributor

@ibelar ibelar commented Apr 29, 2020

Make sure there is no gap between the top and navigation menu.
fix #1156

@codecov
Copy link

codecov bot commented Apr 29, 2020

Codecov Report

Merging #1151 into develop will increase coverage by 0.18%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1151      +/-   ##
=============================================
+ Coverage      75.77%   75.96%   +0.18%     
- Complexity      2538     2539       +1     
=============================================
  Files            130      130              
  Lines           6230     6287      +57     
=============================================
+ Hits            4721     4776      +55     
- Misses          1509     1511       +2     
Impacted Files Coverage Δ Complexity Δ
src/App.php 84.74% <0.00%> (+1.88%) 142.00% <0.00%> (+1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff825b6...514a912. Read the comment docs.

@ibelar ibelar requested a review from DarkSide666 April 30, 2020 13:08
@ibelar ibelar added the Saasty.io 🏗️ Sponsored by Saasty.io - Online ATK app builder label Apr 30, 2020
@ibelar
Copy link
Contributor Author

ibelar commented May 1, 2020

Fix #1156

@mvorisek
Copy link
Member

mvorisek commented May 1, 2020

Fix #1156

Added my CSS rules to that issue, see. And add "fix #nn" text always to the issue description, otherwise it does not work, ie. the issue will be not closed otherwise.

@@ -101,7 +101,7 @@ footer.atk-footer .ui.segment {
margin: 0;
}
.ui.left.sidebar {
z-index: 103;
z-index: 99;
Copy link
Member

Choose a reason for hiding this comment

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

Not a fix to #1156

Copy link
Member

@mvorisek mvorisek May 1, 2020

Choose a reason for hiding this comment

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

Not sure what this should fix on #1156, as it does not solved it it should be probably reverted.

But otherwise PR is read to be merged

@@ -101,7 +101,7 @@ footer.atk-footer .ui.segment {
margin: 0;
}
.ui.left.sidebar {
z-index: 103;
z-index: 99;
Copy link
Member

@mvorisek mvorisek May 1, 2020

Choose a reason for hiding this comment

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

Not sure what this should fix on #1156, as it does not solved it it should be probably reverted.

But otherwise PR is read to be merged

@mvorisek
Copy link
Member

mvorisek commented May 1, 2020

@ibelar #1156 should be fixed, please review carefully and better rerun less build, I edited it manually.

Copy link
Member

@DarkSide666 DarkSide666 left a comment

Choose a reason for hiding this comment

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

Fixes issue for me, so LGTM

@DarkSide666 DarkSide666 merged commit 74052d3 into develop May 2, 2020
@DarkSide666 DarkSide666 deleted the fix/header-menu-size branch May 2, 2020 14:12
@mvorisek
Copy link
Member

mvorisek commented May 2, 2020

Fixes issue for me, so LGTM

@DarkSide666 because I fixed it :)

@DarkSide666
Copy link
Member

Thank you! :)

@mkrecek234
Copy link
Contributor

Still have the same issue, no change: Looks like that (with Maestro theme) - also manually changing the z-index does not have an effect :
image

@ibelar
Copy link
Contributor Author

ibelar commented May 2, 2020

Are you pulling css locally or via jsDeliver? If from jsDeliver, then give it a day at least to be updates correctly.

@mvorisek
Copy link
Member

mvorisek commented May 2, 2020

Are you pulling css locally or via jsDeliver? If from jsDeliver, then give it a day at least to be updates correctly.

PR #1168 for faster collaboration ? @mkrecek234 does this help?

PS using old Admin, Maestro might need more work, please double check/fix my commits if needed.

@mkrecek234
Copy link
Contributor

Works now, probably then was that caching/CDN delay problem. Is it intentional to have a vertical line right to the app header? I think before there was none, probably again subjective, but looked better to me, more as a title, than a menu:
Ohne Titel

@mvorisek
Copy link
Member

mvorisek commented May 3, 2020

@mkrecek234 Great, thank you for your fbk. This PR should have no affect on the horizontal/top panel excep the updated height.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Saasty.io 🏗️ Sponsored by Saasty.io - Online ATK app builder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix admin left menu styling
4 participants