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

[#1976] Ensuring notifications are no longer sticky #919

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

alextreme
Copy link
Member

No description provided.

@alextreme alextreme requested a review from jiromaykin December 29, 2023 13:14
@codecov-commenter
Copy link

codecov-commenter commented Dec 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ca8f7ab) 93.07% compared to head (d675762) 93.07%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #919   +/-   ##
========================================
  Coverage    93.07%   93.07%           
========================================
  Files          828      828           
  Lines        28995    28995           
========================================
  Hits         26988    26988           
  Misses        2007     2007           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -1,5 +1,4 @@
.notifications {
position: sticky;
Copy link
Contributor

Choose a reason for hiding this comment

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

This does mean ALL notifications are no longer sticky, like the approval for newly added contact etc. Do we want this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see a value in having the notifications sticky (I think the only reason we did this was to ensure notifications wouldn't interfere with the rest of the design). Consensus with Groningen is non-sticky and a 'wegklik kruisje' for the post-upload MijnAanvragen notification, however your call if you want to make this change overall or not

Copy link
Contributor

Choose a reason for hiding this comment

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

This PR will need to either be hijacked by me or put on hold for the future as part of researching what the effect will be: https://taiga.maykinmedia.nl/project/open-inwoner/task/1976

Copy link
Member Author

Choose a reason for hiding this comment

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

hijack away or decline, my ego can take it 👍

@jiromaykin jiromaykin self-assigned this Jan 9, 2024
@jiromaykin jiromaykin force-pushed the issue/1976-no-sticky-notifications branch from 3039622 to d675762 Compare January 9, 2024 15:16
@jiromaykin jiromaykin self-requested a review January 9, 2024 15:42
@alextreme alextreme merged commit f6d660b into develop Jan 9, 2024
@alextreme alextreme deleted the issue/1976-no-sticky-notifications branch January 9, 2024 16:49
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.

3 participants