Skip to content

Do not render an empty banner #6732

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 27, 2025
Merged

Do not render an empty banner #6732

merged 1 commit into from
Jun 27, 2025

Conversation

akatsoulas
Copy link
Collaborator

No description provided.

@akatsoulas akatsoulas requested a review from escattone June 23, 2025 11:09
@akatsoulas akatsoulas mentioned this pull request Jun 23, 2025
Closed
escattone
escattone reviewed Jun 23, 2025
View reviewed changes
Copy link
Contributor

@escattone escattone left a comment

Choose a reason for hiding this comment

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

This breaks two things:

  • Announcements can no longer be closed.
  • The data-close-initial="hidden" was needed to make the session-based memory work -- at least with the current implementation of ui.js (

    kitsune/kitsune/sumo/static/sumo/js/ui.js

    Line 86 in 107ead7

    if ($target.data("close-initial") === "hidden") {
    )

From what I can tell from local testing, the flicker is due to the geoip-suggestion announcement, which is initially absent, then shows as blank, and then finally disappears.

@escattone
Copy link
Contributor

escattone commented Jun 23, 2025 •
edited
Loading

My understanding is that Mozilla's location service (which the geoip-suggestion banner functionality relies on) is no longer supported. It was supposed to be shutdown in July of 2024, but it still seems to be running. Perhaps we should disable the geoip-suggestion announcement banner -- which is only used for Indonesia and Bangladesh -- until we can fix it?

@akatsoulas
Copy link
Collaborator Author

The issue was a bit misleading. We don't have a flickering banner but an empty one. By looking closely, the problem is that we render an empty container when the caller() is empty. So we have <p></p> before JS kicks in to hide the container

@akatsoulas akatsoulas changed the title Use css class instead of a data attribute Do not render an empty banner Jun 27, 2025
@akatsoulas akatsoulas merged commit 4fdb9b5 into mozilla:main Jun 27, 2025
2 checks passed
@akatsoulas akatsoulas deleted the flickering-banner branch June 27, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@escattone escattone escattone left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@akatsoulas @escattone