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

Remove leftovers from django-messages-extends #10994

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

humitos
Copy link
Member

@humitos humitos commented Jan 4, 2024

Keep rendering regular one-time Django messages attached to the request, but remove the logic used for sticky/permanent messages from django-messages-extends.

Closes #10988
Related readthedocs/ext-theme#259

Keep rendering regular one-time Django messages attached to the request, but
remove the logic used for sticky/permanent messages from
`django-messages-extends`.

Closes #10988
Related readthedocs/ext-theme#259
@humitos humitos requested a review from a team as a code owner January 4, 2024 11:19
@humitos humitos requested a review from agjohnson January 4, 2024 11:19
humitos added a commit to readthedocs/ext-theme that referenced this pull request Jan 4, 2024
@humitos
Copy link
Member Author

humitos commented Jan 4, 2024

I don't understand why we are having those eslint errors now 🤷🏼

@agjohnson
Copy link
Contributor

Huh, yeah I forgot it was even used here. I wouldn't bother addressing anything with the JS, perhaps it's best to just disable eslint in CI?

@stsewd
Copy link
Member

stsewd commented Jan 4, 2024

Eslint error started happening on #10922, maybe the indentation of the return statement. I've founded useful in the past to catch some errors.

humitos added a commit to readthedocs/ext-theme that referenced this pull request Jan 4, 2024
* Initial comment

* Show new notifications statically via Django templates

* Refresh the page once the build has finished

This temporal hack allows us to show the new notifications from Django
templates. The next step here is to implement the notifications properly using
Knockout and keep this feature on.

* Keep rendering old `Build.error`

Render new `Notification` if there are any. Otherwise, render `Build.error`.

* Iterate over the right element

* Remove debugging

* Lint

* Remove rendering for `django-messages-extends`

Related readthedocs/readthedocs.org#10994

* Remove invalid comment

* Build assets
@humitos
Copy link
Member Author

humitos commented Jan 4, 2024

I will try to address the eslint error in another PR.

@humitos humitos merged commit 5ab5019 into main Jan 4, 2024
5 of 6 checks passed
@humitos humitos deleted the humitos/django-messages-leftovers branch January 4, 2024 18:28
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.

Notifications: migrate old custom Django message notifications
3 participants