Skip to content

Reset top banner local storage entry every 30 days#651

Merged
dgarcia360 merged 2 commits intoscylladb:masterfrom
dgarcia360:set-localstorage-expiration
Dec 13, 2022
Merged

Reset top banner local storage entry every 30 days#651
dgarcia360 merged 2 commits intoscylladb:masterfrom
dgarcia360:set-localstorage-expiration

Conversation

@dgarcia360
Copy link
Copy Markdown
Collaborator

@dgarcia360 dgarcia360 commented Dec 9, 2022

Closes #639

  • Splits the JS file in different modules for better maintenance.
  • Sets an expiry date of 30 days for the top banner.

How to test this PR

  1. Clone this PR.
  2. In docs/conf.py, set html_theme_options.hide_banner to "false".
  3. On terminal window, go to the docs folder.
  4. Run make preview
  5. Open http://localhost:5500/. The site should render without errors.
  6. Close the top banner.
  7. Open the Inspector. In Chrome, right click > Inspect.
  8. Go to Application > Local Storage tab.
  9. Select localhost:5050.
  10. You should see the entry "scylla-docs-hide-banner" with an expiry date.

image

When a page is rendered, the theme checks if the current date is greater than the expiry date to invalidate the entry.

@dgarcia360 dgarcia360 added the enhancement New feature or request label Dec 9, 2022
@dgarcia360 dgarcia360 changed the title Set banner expiry date Reset top banner local storage entry every 30 days Dec 9, 2022
Comment thread docs/source/conf.py Outdated
Comment thread sphinx_scylladb_theme/static/js/main.bundle.js
Comment thread src/js/promo-banner.js

export const onCloseBanner = () => {
$(".promo-banner__close").on("click", function () {
setItemWithExpiry(localStorageKey, "1", 30);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we move this value to config.py?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is possible. If necessary, we can work on it as part of a separate issue.

Copy link
Copy Markdown
Collaborator

@annastuchlik annastuchlik left a comment

Choose a reason for hiding this comment

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

LGTM.
I've followed the testing procedure in the description of this PR.

@annastuchlik
Copy link
Copy Markdown
Collaborator

This PR can be merged.

@dgarcia360 dgarcia360 merged commit 7a260f3 into scylladb:master Dec 13, 2022
@dgarcia360 dgarcia360 deleted the set-localstorage-expiration branch December 13, 2022 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reset top banner cookie every time we update the banner

3 participants