Support (icon) tabs in top navbar

hansegucker commented

2021-11-23 17:05:05 +01:00

Owner

Advances AlekSIS/official/AlekSIS-App-Alsijil#189

Screenshots: https://edugit.org/AlekSIS/official/AlekSIS-App-Alsijil/-/merge_requests/235#note_43799

Advances AlekSIS/official/AlekSIS-App-Alsijil#189 Screenshots: https://edugit.org/AlekSIS/official/AlekSIS-App-Alsijil/-/merge_requests/235#note_43799

hansegucker commented

2021-11-23 17:05:11 +01:00

Author

Owner

requested review from @ZugBahnHof

hansegucker commented

2021-11-23 17:05:11 +01:00

Author

Owner

assigned to @nik

hansegucker commented

2021-11-23 17:05:23 +01:00

Author

Owner

marked this merge request as draft

marked this merge request as **draft**

hansegucker commented

2021-11-23 17:12:40 +01:00

Author

Owner

mentioned in merge request AlekSIS-App-Alsijil!1880

leuckeju commented

2021-11-24 20:56:37 +01:00

Member

is this absolute measurement really necessary? I would suggest to try something relative like 6rem (notice that the number is random) Look e.g. here: https://github.com/materializecss/materialize/blob/main/sass/components/_global.scss#L142 Maybe there is also a fitting preexisting scss variable (although I couldn't find any.

is this absolute measurement really necessary? I would suggest to try something relative like `6rem` (notice that the number is random) Look e.g. here: https://github.com/materializecss/materialize/blob/main/sass/components/_global.scss#L142 Maybe there is also a fitting preexisting scss variable (although I couldn't find any.

leuckeju commented

2021-11-24 20:56:37 +01:00

Member

Please try to find relative measurements or scss variables (look e.g. at https://edugit.org/AlekSIS/official/AlekSIS-Core/-/merge_requests/770/diffs#9c5cc53dd7c47a3ff9897245b2332e0fd71f93e4_771_792)

hansegucker commented

2021-11-24 21:00:26 +01:00

Author

Owner

I thought it would be useful to adapt existing structures as provided by Materialize [1]. Please give me a rationale why not.

[1] https://github.com/materializecss/materialize/blob/main/sass/components/_tabs.scss#L32

I thought it would be useful to adapt existing structures as provided by Materialize [1]. Please give me a rationale why not. [1] https://github.com/materializecss/materialize/blob/main/sass/components/_tabs.scss#L32

hansegucker commented

2021-11-24 21:01:22 +01:00

Author

Owner

As I used official Material design standards which provided the values in pixels, I wouldn't know, how to this in relative units.

leuckeju commented

2021-11-24 21:05:16 +01:00

Member

Then why don't you just use @extend? Or am I missing something?

Then why don't you just use `@extend`? Or am I missing something?

leuckeju commented

2021-11-24 21:05:41 +01:00

Member

Could you provide me a link to your source?

hansegucker commented

2021-11-24 21:06:08 +01:00

Author

Owner

Yes, because these are new numbers.

Yes, because these are *new* numbers.

hansegucker commented

2021-11-24 21:06:50 +01:00

Author

Owner

https://material.io/components/tabs#specs

leuckeju commented

2021-11-24 21:20:49 +01:00

Member

![https://c.tenor.com/BZUMJ52AnZcAAAAC/ill-allow.gif](https://c.tenor.com/BZUMJ52AnZcAAAAC/ill-allow.gif)

leuckeju commented

2021-11-24 21:22:41 +01:00

Member

If I understand your numbers correctly, the tab should have a height of 50px (12px + 14px from a, and 24px from the icon), but it's defined as 48px inside materialize. What do you think about both of them having the same height of 50% of their parent? Then they would be 24px and 24px, but your code only says

I think there would be a better solution to this, but I don't know how. Due to there being no new specification for material tabs, we can do it this way, although I'm not a fan of it.

~~If I understand your numbers correctly, the tab should have a height of 50px (12px + 14px from a, and 24px from the icon), but it's defined as 48px inside materialize. What do you think about both of them having the same height of 50% of their parent? Then they would be 24px and 24px, but your code only says~~ I think there would be a better solution to this, but I don't know how. Due to there being no new specification for material tabs, we can do it this way, although I'm not a fan of it.

leuckeju commented

2021-11-24 21:22:42 +01:00

Member

resolved all threads

leuckeju commented

2021-11-24 21:23:35 +01:00

Member

I think we should note somewhere, why these numbers exist and the need to change them sometime

hansegucker commented

2021-11-25 16:21:41 +01:00

Author

Owner

added 4 commits

50fc14e8...ca96566e - 3 commits from branch master
54f7a90b - Merge branch 'master' into feature/top-nav-tabs

Compare with previous version

added 4 commits <ul><li>50fc14e8...ca96566e - 3 commits from branch <code>master</code></li><li>54f7a90b - Merge branch 'master' into feature/top-nav-tabs</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/789/diffs?diff_id=13718&start_sha=50fc14e869ce76af3ff47f5dd4d301d15e4fa770)

hansegucker commented

2021-11-25 16:21:57 +01:00

Author

Owner

marked this merge request as ready

marked this merge request as **ready**

hansegucker commented

2021-11-28 15:58:59 +01:00

Author

Owner

@nik Please review and merge.

hansegucker commented

2021-11-28 20:44:29 +01:00

Author

Owner

changed the description

nik commented

2021-11-28 20:48:08 +01:00

Owner

Please add how to extend the navbar in the template usage docs.

hansegucker commented

2021-11-28 21:32:55 +01:00

Author

Owner

added 1 commit

a9817e6a - Add some documentation on the extended navbar

Compare with previous version

added 1 commit <ul><li>a9817e6a - Add some documentation on the extended navbar</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/789/diffs?diff_id=13766&start_sha=54f7a90bb0cc1c9a16ced8d58dbe0e464dee4c4e)

hansegucker commented

2021-11-28 21:36:20 +01:00

Author

Owner

added 21 commits

a9817e6a...02f69313 - 19 commits from branch master
f91d1922 - Merge branch 'master' into feature/top-nav-tabs
b61275f6 - Merge branch 'master' into feature/top-nav-tabs

Compare with previous version

added 21 commits <ul><li>a9817e6a...02f69313 - 19 commits from branch <code>master</code></li><li>f91d1922 - Merge branch 'master' into feature/top-nav-tabs</li><li>b61275f6 - Merge branch 'master' into feature/top-nav-tabs</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/789/diffs?diff_id=13772&start_sha=a9817e6a47ef8a1fb461b3cf99d2fa4640d9dbf0)

nik scheduled this pull request to auto merge when all checks succeed

2021-11-28 21:40:06 +01:00

nik merged commit 05bd93b47d into master

2021-11-29 09:50:06 +01:00

nik commented

2021-11-29 09:50:07 +01:00

Owner

mentioned in commit 05bd93b47d

mentioned in commit 05bd93b47d57e3609779fd1ee1d28fea5482ac7f

Rows
Columns

Support (icon) tabs in top navbar #2434