entrouvert
/
publik-base-theme
15
1
Fork 1
Code Issues Pull Requests 11 Releases Wiki Activity

scss: include reponsive menu mask display option (#72654) #87

Merged
smihai merged 1 commits from wip/72654-show-menu-mask-option into main 2022-12-23 10:51:43 +01:00
Conversation 4 Commits 1 Files Changed 3 +25
smihai commented 2022-12-20 17:27:25 +01:00
Owner
Copy Link

Reprise de ce qu'a fait ThomasJ pour le CD06.

Reprise de ce qu'a fait ThomasJ pour le CD06.
tjund requested review from tjund 2022-12-21 10:33:32 +01:00
tjund reviewed 2022-12-21 10:34:43 +01:00
static/includes/_nav.scss Outdated
@ -339,2 +341,4 @@
@if $responsive-menu == left-to-right {
background: transparent;
@if $responsive-menu-use-mask == true {
@media ($max-mobile-viewport) {
tjund commented 2022-12-21 10:34:43 +01:00
Owner
Copy Link

Attention, la nav a son propre breakpoint : $nav-mobile-limit.
Et tu es déjà dans une condition @media screen and (max-width: $nav-mobile-limit) { initié ligne 196

Attention, la nav a son propre breakpoint : $nav-mobile-limit. Et tu es déjà dans une condition `@media screen and (max-width: $nav-mobile-limit) {` initié ligne 196
smihai force-pushed wip/72654-show-menu-mask-option from 47610140eb to 11fcf04c2c 2022-12-21 10:42:45 +01:00 Compare
tjund reviewed 2022-12-21 10:44:23 +01:00
static/includes/_nav.scss Outdated
@ -340,1 +342,4 @@
background: transparent;
@if $responsive-menu-use-mask == true {
@media (max-width: $nav-mobile-limit) {
~ div.gru-nav-mask {
tjund commented 2022-12-21 10:44:23 +01:00
Owner
Copy Link

Je déplacerais ce code après &+ ul pour rester cohérent avec l'organisation présente

		& + ul {
			@if $responsive-menu == top-to-bottom {
				height: auto;
			} @else if $responsive-menu == left-to-right {
				transform: translateX(0);
			}
		}
               ~ div.gru-nav-mask {
                   @if $responsive-menu == left-to-right {
                       …
Je déplacerais ce code après `&+ ul` pour rester cohérent avec l'organisation présente ``` css & + ul { @if $responsive-menu == top-to-bottom { height: auto; } @else if $responsive-menu == left-to-right { transform: translateX(0); } } ~ div.gru-nav-mask { @if $responsive-menu == left-to-right { …
tjund reviewed 2022-12-21 10:48:07 +01:00
static/includes/_nav.scss Outdated
@ -35,6 +35,8 @@ $nav-mobile-menu-item-hover-background: $nav-item-hover-background !default;
$nav-mobile-menu-item-hover-color: $nav-item-hover-color !default;
$nav-item-spacing: 0px !default;
$responsive-menu: top-to-bottom !default; // or left-to-right
$responsive-menu-use-mask: false !default;
tjund commented 2022-12-21 10:48:07 +01:00
Owner
Copy Link

Pourquoi pas simplement $responsive-menu-mask ?

Pourquoi pas simplement `$responsive-menu-mask` ?
smihai commented 2022-12-21 10:54:04 +01:00
Author
Owner
Copy Link

Pourquoi pas simplement $responsive-menu-mask ?

C'est ma pérception de la chose: comme c'est un booléen il faut un verbe.
Mais ça me va de faire sans.

> Pourquoi pas simplement `$responsive-menu-mask` ? C'est ma pérception de la chose: comme c'est un booléen il faut un verbe. Mais ça me va de faire sans.
tjund marked this conversation as resolved
tjund reviewed 2022-12-21 10:48:40 +01:00
static/includes/_nav.scss Outdated
@ -341,0 +343,4 @@
@if $responsive-menu-use-mask == true {
@media (max-width: $nav-mobile-limit) {
~ div.gru-nav-mask {
display: block;
tjund commented 2022-12-21 10:48:40 +01:00
Owner
Copy Link

Pourquoi display:block, je ne vois nul part un display:none avant.

Pourquoi `display:block`, je ne vois nul part un `display:none` avant.
tjund marked this conversation as resolved
smihai force-pushed wip/72654-show-menu-mask-option from 11fcf04c2c to 7d7a211211 2022-12-21 10:57:37 +01:00 Compare
smihai commented 2022-12-21 10:57:59 +01:00
Author
Owner
Copy Link

Remarques prises en compte.

Remarques prises en compte.
tjund requested changes 2022-12-22 10:40:50 +01:00
static/includes/_nav.scss Outdated
@ -362,6 +364,19 @@ div.menucell {
} @else if $responsive-menu == left-to-right {
transform: translateX(0);
}
@if $responsive-menu-mask == true {
tjund commented 2022-12-21 17:21:39 +01:00
Owner
Copy Link

Je proposais de positionner en dehors de ul, juste après

Et il manque la condition left-to-right

@if ($responsive-menu-mask == true and $responsive-menu == left-to-right)

Je proposais de positionner en dehors de ul, juste après Et il manque la condition left-to-right `@if ($responsive-menu-mask == true and $responsive-menu == left-to-right)`
static/includes/_nav.scss Outdated
@ -363,2 +365,4 @@
transform: translateX(0);
}
@if $responsive-menu-mask == true {
@media (max-width: $nav-mobile-limit) {
tjund commented 2022-12-21 17:16:25 +01:00
Owner
Copy Link

À supprimer, (c'est inutile tu es déjà dans une condition nav mobile)

À supprimer, (c'est inutile tu es déjà dans une condition nav mobile)
smihai force-pushed wip/72654-show-menu-mask-option from 7d7a211211 to 22dd788aae 2022-12-22 10:57:38 +01:00 Compare
smihai commented 2022-12-22 10:58:00 +01:00
Author
Owner
Copy Link

Remarques prises en compte.

Remarques prises en compte.
tjund approved these changes 2022-12-22 14:41:17 +01:00
smihai merged commit e3a3970775 into main 2022-12-23 10:51:43 +01:00
smihai deleted branch wip/72654-show-menu-mask-option 2022-12-23 10:51:43 +01:00
fpeters commented 2023-01-02 18:06:18 +01:00
Owner
Copy Link

Pourquoi c'est une option et pas juste l'unique comportement ?

Pourquoi c'est une option et pas juste l'unique comportement ?
smihai commented 2023-01-03 09:58:00 +01:00
Author
Owner
Copy Link

Pourquoi c'est une option et pas juste l'unique comportement ?

Par méfiance je préfère que ce soit une option désactivée par défaut.

> Pourquoi c'est une option et pas juste l'unique comportement ? Par méfiance je préfère que ce soit une option désactivée par défaut.
Sign in to join this conversation.
Reviewers
No reviewers
tjund
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/publik-base-theme#87
No description provided.
Delete Branch "wip/72654-show-menu-mask-option"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?