[WIP] Rewrite navbar #1154

jonah · 2019-08-13T11:36:20Z

dawidpotocki commented

2019-08-13 11:36:20 +00:00

(Migrated from github.com)

Fixes #877, Fixes #872, Closes #1195.

No Javascript needed, that's why I even started it :)

Dropdowns
Hide dropdown when clicking anything else
Mobile Hamburger

Fixes #877, Fixes #872, Closes #1195. No Javascript needed, that's why I even started it :) - [x] Dropdowns - [x] Hide dropdown when clicking anything else - [x] Mobile Hamburger ![image](https://user-images.githubusercontent.com/38681822/62938226-24c77c80-bdbe-11e9-8ace-9eeb34ab9f65.png)

netlify[bot] commented

2019-08-13 11:37:06 +00:00

(Migrated from github.com)

Deploy preview for privacytools-io ready!

Built with commit 3a785fd18b

https://deploy-preview-1154--privacytools-io.netlify.com

Deploy preview for *privacytools-io* ready! Built with commit 3a785fd18b26440117ed2d72ef3c1687b1edfe1f https://deploy-preview-1154--privacytools-io.netlify.com

Mikaela (Migrated from github.com) reviewed 2019-08-13 19:20:34 +00:00

Mikaela (Migrated from github.com) left a comment

I don't understand the code, but it seems to work even if it's a bit funny how everything can be opened simultaneously.

dawidpotocki commented

2019-08-13 20:35:02 +00:00

(Migrated from github.com)

I don't understand the code, but it seems to work even if it's a bit funny how everything can be opened simultaneously.

<details>
	<summary>
		This is summary!
	</summary>
	This is some text
</details>

EDIT: GitHub doesn't like my Markdown, so you can check example there
with output text with arrow next to it:
https://gitlab.com/dawidpotocki/policense#add-email-e-email

Now, how I made this thing ^ work like a dropdown? I had to first create
a <span> after </summary> and make position: absoluteso it would "fly" and not show itself in the navbar, which would destroy everything. Next to fix bug with "Services" dropdown being cut of, I had to set

topositon: relativeso dropdown is shown next to it and thenright: 0` to the dropdown so it would go to the left side.

It's pretty simple, considering that one from BS4 is using Javascript
for the same thing and everything you need is builtin to the HTML!

> I don't understand the code, but it seems to work even if it's a bit funny how everything can be opened simultaneously. ``` <details> <summary> This is summary! </summary> This is some text </details> ``` EDIT: GitHub doesn't like my Markdown, so you can check example there with `output` text with arrow next to it: https://gitlab.com/dawidpotocki/policense#add-email-e-email Now, how I made this thing ^ work like a dropdown? I had to first create a `<span>` after `</summary> and make `position: absolute` so it would "fly" and not show itself in the navbar, which would destroy everything. Next to fix bug with "Services" dropdown being cut of, I had to set `<details>` to `positon: relative` so dropdown is shown next to it and then `right: 0` to the dropdown so it would go to the left side. It's pretty simple, considering that one from BS4 is using Javascript for the same thing and everything you need is builtin to the HTML!

nitrohorse (Migrated from github.com) reviewed 2019-08-14 01:23:25 +00:00

assets/css/style.scss

nitrohorse (Migrated from github.com) commented

2019-08-14 01:23:25 +00:00

Small suggestion, since we're using SASS could we use color variables for these new classes to allow the colors to be re-usable? I think like this:

$color-gray: #9a9da0;

.nav-text {
  color: $color-gray;
}

Small suggestion, since we're using SASS could we use color variables for these new classes to allow the colors to be re-usable? I think like this: ``` $color-gray: #9a9da0; .nav-text { color: $color-gray; } ```

nitrohorse commented

2019-08-14 01:24:19 +00:00

(Migrated from github.com)

This is really cool! Though I think we'll need Hide dropdown when clicking anything else to be resolved; the usability without it isn't great.

This is really cool! Though I think we'll need `Hide dropdown when clicking anything else` to be resolved; the usability without it isn't great.

👍 1

dawidpotocki (Migrated from github.com) reviewed 2019-08-14 07:59:47 +00:00

assets/css/style.scss

dawidpotocki (Migrated from github.com) commented

2019-08-14 07:59:47 +00:00

I don't think that there is need for that as it probably will not be used anywhere else.
It was the color used before in navbar, that was probably built-in to BS4.
But if you want, I can do that.

I don't think that there is need for that as it probably will not be used anywhere else. It was the color used before in navbar, that was probably built-in to BS4. But if you want, I can do that.

nitrohorse (Migrated from github.com) reviewed 2019-08-14 14:21:57 +00:00

assets/css/style.scss

nitrohorse (Migrated from github.com) commented

2019-08-14 14:21:56 +00:00

Ah, that makes sense. I suppose we can make the colors re-usable if we need to reference them again in the future somewhere else.

dawidpotocki commented

2019-08-15 09:49:42 +00:00

(Migrated from github.com)

Though I think we'll need Hide dropdown when clicking anything else to be resolved; the usability without it isn't great.

Got it working with JS. I don't believe it is possible without it. But at least navbar dropdowns sort of work, which was not the case before.

> Though I think we'll need Hide dropdown when clicking anything else to be resolved; the usability without it isn't great. Got it working with JS. I don't believe it is possible without it. But at least navbar dropdowns _sort of work_, which was not the case before.

jonah reviewed 2019-08-16 21:30:53 +00:00

assets/css/style.scss

jonah commented

2019-08-16 21:30:53 +00:00

This :hover color seemingly does not apply to navbar items without dropdowns (i.e. the home button, participate link, and donate link).

This `:hover` color seemingly does not apply to navbar items without dropdowns (i.e. the home button, participate link, and donate link).

jonah reviewed 2019-08-16 22:28:10 +00:00

assets/css/style.scss

jonah commented

2019-08-16 22:28:09 +00:00

The actual old navbar color is rgba(255,255,255,0.5), FYI

The actual old navbar color is `rgba(255,255,255,0.5)`, FYI

dawidpotocki (Migrated from github.com) reviewed 2019-08-17 02:07:28 +00:00

assets/css/style.scss

dawidpotocki (Migrated from github.com) commented

2019-08-17 02:07:27 +00:00

It is because I renamed .nav-link to .nav-anchor, because it was
already defined by Bootstrap, so I renamed it and forgot to update it.
Should be good now.

(if you got the same reply to your email twice, sorry but GitHub does not like me replying to these comments with email :C)

It is because I renamed `.nav-link` to `.nav-anchor`, because it was already defined by Bootstrap, so I renamed it and forgot to update it. Should be good now. (if you got the same reply to your email twice, sorry but GitHub does not like me replying to these comments with email :C)

Perelandra0x309 commented

2019-08-19 21:34:19 +00:00

(Migrated from github.com)

W3.CSS does nice navbars with no JS.
https://www.w3schools.com/w3css/w3css_navigation.asp

W3.CSS does nice navbars with no JS. https://www.w3schools.com/w3css/w3css_navigation.asp

dawidpotocki commented

2019-08-19 21:59:03 +00:00

(Migrated from github.com)

W3.CSS does nice navbars with no JS.

They are on hover, this one is on click. The collapsing one is using
JavaScript, so it's not really useful.

> W3.CSS does nice navbars with no JS. They are on hover, this one is on click. The collapsing one is using JavaScript, so it's not really useful.

Perelandra0x309 commented

2019-08-20 00:09:13 +00:00

(Migrated from github.com)

They do have clickable ones too. The menu on that website itself is a clickable menu, so can be used as an example.

dawidpotocki commented

2019-08-20 00:21:32 +00:00

(Migrated from github.com)

They do have clickable ones too. The menu on that website itself is a clickable menu, so can be used as an example.

They. Require. JavaScript.

> They do have clickable ones too. The menu on that website itself is a clickable menu, so can be used as an example. They. Require. JavaScript.

😆 1

jonah commented

2019-08-20 02:51:06 +00:00

This is coming along great! I wonder if you would be able to push the dropdown box down a bit...

Old:

Current in PR:

For some reason that dropdown being so close to the navbar text is unsettling to me. Also, want to make sure changing the color of an active dropdown to white is on your to-do list. I would imagine summary:focus would do the trick, maybe not perfectly but good enough, unless you have other plans.

This is coming along great! I wonder if you would be able to push the dropdown box down a bit... Old: <img width="391" alt="image" src="https://user-images.githubusercontent.com/3637842/63313388-d6d4dc00-c2c9-11e9-827a-957e0fd4b094.png"> Current in PR: <img width="391" alt="image" src="https://jda.mn/z5rqw"> For some reason that dropdown being so close to the navbar text is unsettling to me. Also, want to make sure changing the color of an active dropdown to white is on your to-do list. I would imagine `summary:focus` would do the trick, maybe not perfectly but good enough, unless you have other plans.

👍 1

dawidpotocki commented

2019-08-20 04:07:23 +00:00

(Migrated from github.com)

This is coming along great! I wonder if you would be able to push the dropdown box down a bit...
For some reason that dropdown being so close to the navbar text is unsettling to me. Also, want to make sure changing the color of an active dropdown to white is on your to-do listw I would imagine summary:focus would do the trick, maybe not perfectly but good enough, unless you have other plans.

Yeah, I'm planning to do that, but my priority is making a mobile
hamburger ui now. I'm trying to implement it without JavaScript and it
seems to be possible, all the possible ways are weird hacks, but
whatever.

> This is coming along great! I wonder if you would be able to push the dropdown box down a bit... > For some reason that dropdown being so close to the navbar text is unsettling to me. Also, want to make sure changing the color of an active dropdown to white is on your to-do listw I would imagine `summary:focus` would do the trick, maybe not perfectly but good enough, unless you have other plans. Yeah, I'm planning to do that, but my priority is making a mobile hamburger ui now. I'm trying to implement it without JavaScript and it seems to be possible, all the possible ways are weird hacks, but whatever.

❤️ 1

Perelandra0x309 commented

2019-08-20 10:27:56 +00:00

(Migrated from github.com)

They. Require. JavaScript.

Where do you see that? I am using it just fine without JS.

> They. Require. JavaScript. Where do you see that? I am using it just fine without JS.

dawidpotocki commented

2019-08-20 12:54:44 +00:00

(Migrated from github.com)

Where do you see that? I am using it just fine without JS.

https://invidio.us/watch?v=mNttgpgNFS4

> Where do you see that? I am using it just fine without JS. https://invidio.us/watch?v=mNttgpgNFS4

Perelandra0x309 commented

2019-08-21 10:14:08 +00:00

(Migrated from github.com)

If you insist on having clickable menus (on desktop) then yes JS in needed for that one feature. But I thought you wanted this to be JS free? Are hoverable menus that bad? Those do not require JS. Also the hoverable menus from W3 are actually clickable on mobile devices.

BTW W3 is just a framework of standard CSS attributes, just makes it easier to do what you want. You are probably doing the exact same with with your straight CSS if you are able to get hoverable menus without JS. So it's a decision you and the site owners need to make- use JS to get clickable menus or drop JS and have menus be hoverable on desktop (clickable on mobiles).

Edit:
Also consider usability of the website for people who choose to disable JS in their browser.

If you insist on having clickable menus (on desktop) then yes JS in needed for that one feature. But I thought you wanted this to be JS free? Are hoverable menus that bad? Those do not require JS. Also the hoverable menus from W3 are actually clickable on mobile devices. BTW W3 is just a framework of standard CSS attributes, just makes it easier to do what you want. You are probably doing the exact same with with your straight CSS if you are able to get hoverable menus without JS. So it's a decision you and the site owners need to make- use JS to get clickable menus or drop JS and have menus be hoverable on desktop (clickable on mobiles). Edit: Also consider usability of the website for people who choose to disable JS in their browser.

👍 1

dawidpotocki commented

2019-08-21 10:22:26 +00:00

(Migrated from github.com)

If you insist on having clickable menus (on desktop) then yes JS in needed for that one feature.

But. I. Implemented. Them. In. This. Pull. Request. Without. JavaScript.

> If you insist on having clickable menus (on desktop) then yes JS in needed for that one feature. But. I. Implemented. Them. In. This. Pull. Request. Without. JavaScript.

Perelandra0x309 commented

2019-08-21 10:45:18 +00:00

(Migrated from github.com)

Ah, OK your last update from yesterday indicated you were having to use "weird hacks" to do it without JS. If you got it working, great!

dawidpotocki commented

2019-08-22 20:22:27 +00:00

(Migrated from github.com)

Everything is finished.

I think.

__Everything is finished.__ I think.

Mikaela (Migrated from github.com) approved these changes 2019-08-22 21:24:28 +00:00

Mikaela (Migrated from github.com) left a comment

I am staying with my previous comment:

I don't understand the code, but it seems to work even if it's a bit funny how everything can be opened simultaneously.

And clarify that everything cannot be opened simultaneously unless scripts are blocked with µMatrix

I am staying with my previous comment: > I don't understand the code, but it seems to work even if it's a bit funny how everything can be opened simultaneously. And clarify that everything cannot be opened simultaneously unless scripts are blocked with µMatrix

jonah approved these changes 2019-08-22 22:19:05 +00:00

jonah left a comment

This... appears to work.

This repo is archived. You cannot comment on pull requests.

No reviewers

Mikaela