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

Store volume level on window object #2307

Conversation

ismailkarsli
Copy link
Contributor

Description

Currently every video starts muted. With this PR, the volume value is kept in the window and used while playing other videos.

componentDidMount is formatted

Husky auto formatting line 131 while commiting.

@alectrocute
Copy link
Contributor

I'd suggest using window.localStorage.[getItem/setItem] instead to persist. Let's avoid adding things to the window object.

@ismailkarsli
Copy link
Contributor Author

I'd suggest using window.localStorage.[getItem/setItem] instead to persist. Let's avoid adding things to the window object.

I specifically did it this way so that the volume value starts muted in each session. However, if you suggest otherwise, I can do it with localStorage (or another global store if available).

@ismailkarsli
Copy link
Contributor Author

Alright I've changed it to work with localStorage.

Copy link
Member

@SleeplessOne1917 SleeplessOne1917 left a comment

Choose a reason for hiding this comment

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

Besides the changes requested from my other comments, please run prettier. It looks like some formatting changed.

@ismailkarsli
Copy link
Contributor Author

Besides the changes requested from my other comments, please run prettier. It looks like some formatting changed.

I've run prettier (in fact it runs with pre-commit hook anyways) but that part of the code still stays formatted. I guess it was not formatted according to the config before.

image

@ismailkarsli
Copy link
Contributor Author

I've checked it and event.target is undefined when used with linkEvent. Lemmy fix that

@SleeplessOne1917 SleeplessOne1917 merged commit 0bd26aa into LemmyNet:main Jan 3, 2024
1 check passed
@ismailkarsli ismailkarsli deleted the store-volume-level-on-window-object branch January 3, 2024 20:31
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.

4 participants