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

Added valentine growl #6657

Merged
merged 2 commits into from
Feb 8, 2024
Merged

Added valentine growl #6657

merged 2 commits into from
Feb 8, 2024

Conversation

mzparacha
Copy link
Contributor

@mzparacha mzparacha commented Feb 8, 2024

Link to Issue

Closes: #6636

Description of Changes

  • Added Valentine's growl
image

"How We Fixed It"

N/A

Test Plan

  • Visit any page
  • Verify you see the valentine's growl
  • Click on the 'Add to Calender' button, verify it opens OS calender and adds event entry.
  • Try to close it from the x button, verify it closes and shows again on page reload
  • Click on the "Don't show this again" checkbox, and close it again from the x button, verify it closes and doesn't show again on page reload.

Deployment Plan

N/A

Other Considerations

N/A

@mzparacha mzparacha marked this pull request as ready for review February 8, 2024 17:41
@mzparacha mzparacha requested review from lzach83 and masvelio and removed request for lzach83 February 8, 2024 17:43
@mzparacha mzparacha self-assigned this Feb 8, 2024
Copy link
Contributor

@CowMuon CowMuon left a comment

Choose a reason for hiding this comment

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

Really appreciate the detail level in testing instructions, @mzparacha
This appears to do the thing- can we ship this, with gating growl removal 6517 as patch 0.9.4-4 @ilijabojanovic

Copy link
Contributor

@kurtisassad kurtisassad left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Collaborator

@ilijabojanovic ilijabojanovic left a comment

Choose a reason for hiding this comment

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

From UX perspective everything is ok. How ever i have issue to add calendar invite to my local calendar. We should generate calendar files.

Screenshot 2024-02-08 at 18 57 53 Screenshot 2024-02-08 at 18 57 50

We should generate files and allow users to manually add them in calendar.

@mzparacha
Copy link
Contributor Author

@ilijabojanovic can you try on port 8080?

@ilijabojanovic
Copy link
Collaborator

@ilijabojanovic can you try on port 8080?

Still same on 8080.

@kurtisassad
Copy link
Contributor

For what its worth it works for me, but mine goes through google calendar. Maybe it breaks on the macos calendar app?

@mzparacha
Copy link
Contributor Author

Works for me @ilijabojanovic, I am not sure what the issue on your side is.

test.mp4

@CowMuon
Copy link
Contributor

CowMuon commented Feb 8, 2024

Approved to ship, but good catch @ilijabojanovic, this was exactly what we were discussing in parking lot, that it's not uncommon for cal invite handling to have a lacuna between web calendars and locally installed calendar apps. We said on that call we're good with users having to take extra steps.

I haven't tested on Frick or Frack tho it looks like Ilija's error is local only.

@mzparacha
Copy link
Contributor Author

Pushed to frack for testing

cc: @CowMuon @ilijabojanovic

Copy link
Collaborator

@ilijabojanovic ilijabojanovic left a comment

Choose a reason for hiding this comment

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

LGTM

Verified on frack

@mzparacha mzparacha merged commit 98c35b5 into master Feb 8, 2024
6 of 7 checks passed
@mzparacha mzparacha deleted the malik.6636.valentine-growl branch February 8, 2024 18:58
jnaviask pushed a commit that referenced this pull request Feb 8, 2024
* Added valentine growl

* Added invite info
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Growl" for Valentine's Community Call
5 participants