C24_WMDE_Mobile_DE_09 #624
Conversation
moiikana
force-pushed
the
C24_WMDE_Mobile_DE_09
branch
2 times, most recently
from
2431a6b
to
1e19bb6
Compare
- copy ctrl files
moiikana
force-pushed
the
C24_WMDE_Mobile_DE_09
branch
8 times, most recently
from
cc7ed54
to
2f9ad23
Compare
moiikana
force-pushed
the
C24_WMDE_Mobile_DE_09
branch
11 times, most recently
from
03af03b
to
2219d12
Compare
moiikana
force-pushed
the
C24_WMDE_Mobile_DE_09
branch
from
2219d12
to
41c4003
Compare
banners/mobile/C24_WMDE_Mobile_DE_09/styles/MiniBanner_var.scss
Outdated
Show resolved
Hide resolved
| @@ -1,7 +1,7 @@ | |||
| <template> | |||
| <dialog class="wmde-banner-10-reasons" ref="reasonsDialogue"> | |||
| <div class="wmde-banner-10-reasons-close"> | |||
| <button @click="$emit( 'hide' )"> | |||
There was a problem hiding this comment.
I like the simpler event names that don't repeat the component name in them, they'll be easier to read in the template. But that's just my personal taste.
There was a problem hiding this comment.
I see that you're trying to make this more consistent with the Use of Funds, but I'd prefer if this component fired different events for the close and CTA click, as this removes the need for an enum, and the requirement for an if branch in all the banner components.
There was a problem hiding this comment.
I like the simpler event names that don't repeat the component name in them, they'll be easier to read in the template. But that's just my personal taste.
I hope I understood your point, but if we e.g. have 3 popup features that all send "hide" to the parent,
I feared that the browser / DOM event handling could be messing up which event came from which component.
If that's not the case and events stay within their "contact way to the parent" then we can have simpler event names, yes.
There was a problem hiding this comment.
Vue emits aren't generic browser events, and it binds the handlers directly (eg @hide="doThisThing"). So 3 components can all emit "hide" events, and as long as they're bound to different functions it'll be okay.
| @@ -1,7 +1,7 @@ | |||
| <template> | |||
| <dialog class="wmde-banner-10-reasons" ref="reasonsDialogue"> | |||
| <div class="wmde-banner-10-reasons-close"> | |||
| <button @click="$emit( 'hide' )"> | |||
There was a problem hiding this comment.
I see that you're trying to make this more consistent with the Use of Funds, but I'd prefer if this component fired different events for the close and CTA click, as this removes the need for an enum, and the requirement for an if branch in all the banner components.
moiikana
force-pushed
the
C24_WMDE_Mobile_DE_09
branch
from
41c4003
to
e3fdd2a
Compare
- var uses 10 reasons popup - tracks shown, cta-click and accordion item clicks - make minibanners the same size - remove old MinimisedTracker (leftover) - add tests for tracking https://phabricator.wikimedia.org/T379953
moiikana
force-pushed
the
C24_WMDE_Mobile_DE_09
branch
from
e3fdd2a
to
bdb7245
Compare
|
I created a follow up PR that adds explicit styles for the links in the content, and swaps the links to buttons #626 |
This adds some styles so we can use buttons in the slider and full page content in the mobile de banners. Ticket: https://phabricator.wikimedia.org/T379953
- is based on the control banner of desktop-de-13 Ticket: https://phabricator.wikimedia.org/T379950
- adds additional copy - displays the "10 good reasons" overlay when the link in the added copy is clicked - adds styles for the "10 good reasons" button link Ticket: https://phabricator.wikimedia.org/T379953
C24_WMDE_Desktop_DE_14
| Dann entscheiden Sie sich, eine der seltenen Ausnahmen zu sein, und geben Sie etwas zurück. | ||
| Falls Sie zögern, | ||
| <a | ||
| id="reasons-to-donate-link" |
There was a problem hiding this comment.
You can remove this id, it's appearing on the page twice
| Falls Sie zögern, | ||
| <a | ||
| id="reasons-to-donate-link" | ||
| class="wmde-banner-reasons-to-donate-link t-reasons-to-donate-link" |
There was a problem hiding this comment.
| class="wmde-banner-reasons-to-donate-link t-reasons-to-donate-link" | |
| class="wmde-banner-reasons-to-donate-link" |
| @@ -1,7 +1,7 @@ | |||
| <template> | |||
| <dialog class="wmde-banner-10-reasons" ref="reasonsDialogue"> | |||
| <div class="wmde-banner-10-reasons-close"> | |||
| <button @click="$emit( 'hide' )"> | |||
There was a problem hiding this comment.
Vue emits aren't generic browser events, and it binds the handlers directly (eg @hide="doThisThing"). So 3 components can all emit "hide" events, and as long as they're bound to different functions it'll be okay.
based on 08 VAR
var has 10 reasons popup
and tracks
The overlay is displayed.
One of the reasons was expanded (and which).
The button at the bottom of the overlay is clicked.
https://phabricator.wikimedia.org/T379953