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

C24_WMDE_iPad_DE_01 #569

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

C24_WMDE_iPad_DE_01 #569

wants to merge 3 commits into from

Conversation

Sperling-0
Copy link
Contributor

@Sperling-0 Sperling-0 commented Oct 8, 2024

@Sperling-0 Sperling-0 force-pushed the C24_WMDE_iPad_DE_01 branch 2 times, most recently from eb4915a to 5cdbbad Compare October 8, 2024 14:01
@Sperling-0 Sperling-0 marked this pull request as ready for review October 8, 2024 14:25
@Sperling-0 Sperling-0 force-pushed the C24_WMDE_iPad_DE_01 branch 5 times, most recently from d4f4209 to 53c3d27 Compare October 14, 2024 19:31
Copy link
Member

@Abban Abban left a comment

Choose a reason for hiding this comment

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

It's looking good, just needs a few tweaks.

@@ -5,12 +5,13 @@
$select-group-button: true,
Copy link
Member

Choose a reason for hiding this comment

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

When #579 is merged the payment logos here will break. You should be able to already add $select-group-image-label: true to this list.

);
@use '@src/components/BannerConductor/banner-transition';
@use 'Banner';
@use 'MainBanner' with (
$banner-height: 378px,
$form-width: 256px
$form-width: 35%
Copy link
Member

Choose a reason for hiding this comment

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

Pixel width is used here is so the form remains consistent on all screen sizes. Is there a reason you changed it to a percentage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not change it. It came from styles_var.scss of C24_WMDE_iPad_00.

@@ -42,7 +42,7 @@ $progress-bar-margin: 0 !default;
&-fill-wrapper {
position: relative;
background: var( --progress-bar-wrapper-background );
border: 3px var( --progress-bar-wrapper-border );
border: 3px var( --progress-bar-wrapper-border ) solid;
Copy link
Member

Choose a reason for hiding this comment

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

This works, but the general convention for the order when defining border styles is border: <size> <style> <colour>;

border: 3px solid var( --progress-bar-wrapper-border );

Copy link
Member

Choose a reason for hiding this comment

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

This is identical to messages.js so you can delete this and use the same file in both banners.

@Sperling-0
Copy link
Contributor Author

⚠️ Rebase this PR from latest main.

To get the latest UoF working:

  • For Desktop banners: BannerCtrl.vue, BannerVar.vue, FallbackBanner.vue needs to be updated. (If it is DE desktop include DE infographic and vice versa)
  • For Mobile/ Ipad banners: BannerCtrl.vue, BannerVar.vue needs to be updated. (If it is DE mobile include DE infographic and vice versa)

@Sperling-0 Sperling-0 force-pushed the C24_WMDE_iPad_DE_01 branch 2 times, most recently from ff998ac to c2d0db9 Compare October 25, 2024 12:00
@Sperling-0
Copy link
Contributor Author

⚠️ Rebase this PR from latest main.

To get the latest UoF working:

* For Desktop banners: `BannerCtrl.vue`, `BannerVar.vue`, `FallbackBanner.vue` needs to be updated. (If it is DE desktop include DE infographic and vice versa)

* For Mobile/ Ipad banners: `BannerCtrl.vue`, `BannerVar.vue` needs to be updated. (If it is DE mobile include DE infographic and vice versa)

✅ Done

@Sperling-0 Sperling-0 force-pushed the C24_WMDE_iPad_DE_01 branch 10 times, most recently from f96a268 to 06d55bf Compare October 25, 2024 13:34
@moiikana
Copy link
Contributor

consider rebasing because #597 got merged

@Sperling-0 Sperling-0 force-pushed the C24_WMDE_iPad_DE_01 branch 8 times, most recently from a3cf5c5 to 491ca47 Compare October 25, 2024 14:35
Sperling-0 and others added 3 commits October 25, 2024 16:38
- The average donation should be changed from 22,25 € to 22,49 €
- Added space between 99% -> 99 %
- The third option below the two buttons "Ja, ich möchte jährlich spenden, aber einen anderen Betrag." is removed from the 2nd form page.
- The first slide shows the data and time "{{ liveDateAndTime.currentDate }}, {{ liveDateAndTime.currentTime }} - An alle, die Wikipedia in Deutschland nutzen:" and ends with "Unabhängigkeit von Wikipedia zu unterstützen."
- It has the progress bar
- It has the campaign day sentence
- It has the visitors-vs-donors sentence
- It has the soft close
- It has an additional paragraph on the 2nd form page
- It has "I already donated" button in the soft close, respective theme css files have been modified
- Update clean-up-2025 document

Ticket: https://phabricator.wikimedia.org/T376593

Co-authored-by: Abban Dunne <[email protected]>
- The average donation should be changed from 22,25 € to 22,49 €
- Added space between 99% -> 99 %
- The third option below the two buttons "Ja, ich möchte jährlich spenden, aber einen anderen Betrag." is removed from the 2nd form page.
- The first slide shows the data and time "{{ liveDateAndTime.currentDate }}, {{ liveDateAndTime.currentTime }} - An alle, die Wikipedia in Deutschland nutzen:" and ends with "Unabhängigkeit von Wikipedia zu unterstützen."
- The headline of the variant banner is changed
- The amount on the 3rd slide should also be changed from 5 € to 15 €
- The second amount option in the variant banner form should be 15 €
- It has the progress bar
- It has the campaign day sentence
- It has the visitors-vs-donors sentence
- It has "I already donated" button in the soft close
- Update content repo

Ticket: https://phabricator.wikimedia.org/T376593

Co-authored-by: Abban Dunne <[email protected]>
@Sperling-0
Copy link
Contributor Author

consider rebasing because #597 got merged

✅ Done

@Sperling-0 Sperling-0 requested a review from Abban October 25, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants