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.

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

Make certain controls non-selectable / non-draggable - Part II #3957

Merged
merged 3 commits into from
Aug 28, 2023
Merged

Make certain controls non-selectable / non-draggable - Part II #3957

merged 3 commits into from
Aug 28, 2023

Conversation

kommunarr
Copy link
Collaborator

@kommunarr kommunarr commented Aug 27, 2023 •
edited
Loading

Make certain controls non-selectable / non-draggable - Part II

Pull Request Type

  • Bugfix

Related issue

See comment

Description

A previous PR making certain controls non-selectable / non-draggable was thwarted by my inclusion of a user-select: all to unset instead of a user-select: unset. This resolves the issue. Read more about it here.

Testing

Tested on Watch route and other main routes.

Desktop

  • OS: OpenSUSE Tumbleweed
  • OS Version: 2023xxxx
  • FreeTube version: 0.19.0

@kommunarr
This is due to my previous change which included the use of a poorly…
09858af 
… researched 'user-select: all' to unset a 'user-select'. The problem is that all does something else altogether: 'The content of the element shall be selected atomically: If a selection would contain part of the element, then the selection must contain the entire element including all its descendants. If a double-click or context-click occurred in sub-elements, the highest ancestor with this value will be selected.' Source: https://developer.mozilla.org/en-US/docs/Web/CSS/user-select
@kommunarr
Add user-select:none to one more area
2c1c431
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) August 27, 2023 23:46
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Aug 27, 2023
@PikachuEXE
Copy link
Collaborator

I don't really know what should be selectable or not now ._.

@kommunarr
Copy link
Collaborator Author

I don't really know what should be selectable or not now ._.

It's generally not that important on what should or shouldn't be selectable; control elements generally shouldn't be unless they have a label that might need to be searched for whatever reason. It's a question of "does this affect the user experience" (e.g., does trying to click on something sometimes try to select something that just doesn't need to be selected, like the Settings tab labels) and "does this look really weird if I'm trying to select something else and this gets copied" (This was definitely the side nav labels, the ft-input placeholders, and arguably the profile buttons). Beyond that, it's not worth getting stuck in the weeds of the abstract ontological question of "can we put a user-select: none on X?" If you want to know why, just try doing a Ctrl+A on any YouTube page. After this PR, we should be above par compared to them.

PikachuEXE
PikachuEXE previously approved these changes Aug 28, 2023
View reviewed changes
@PikachuEXE PikachuEXE mentioned this pull request Aug 28, 2023
Merged
1 task
@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented Aug 28, 2023 •
edited
Loading

Not sure if it belongs here but i thought it was weird i could do this

VirtualBoxVM_1tMAIMRRjI.mp4

Edit: NVM im stupid

2nd Edit: Also its good that u include in testing what u tested but its also important to lay out to us what we need to test and how NVM this because already mentioned in #3946 (comment)

@FreeTubeBot FreeTubeBot merged commit ce3de2b into FreeTubeApp:development Aug 28, 2023
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Aug 28, 2023
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Aug 29, 2023
@PikachuEXE
Merge branch 'development' into feature/playlist-2023-05
4b21e75 
* development: (65 commits)
  Make constants imported from @fortawesome/free-solid-svg-icons ordered by name again (FreeTubeApp#3958)
  Replace labeler workflow with GH labeler (FreeTubeApp#3966)
  Fix HTML styling (e.g., hashtag links) showing up as raw HTML in descriptions (FreeTubeApp#3946)
  Make certain controls non-selectable / non-draggable - Part II (FreeTubeApp#3957)
  Translated using Weblate (German)
  Show video list buttons on hover or focus (FreeTubeApp#3954)
  Bump @babel/eslint-parser from 7.22.10 to 7.22.11 (FreeTubeApp#3962)
  Bump youtubei.js from 6.0.0 to 6.1.0 (FreeTubeApp#3965)
  Bump eslint-plugin-n from 16.0.1 to 16.0.2 (FreeTubeApp#3964)
  Bump eslint from 8.47.0 to 8.48.0 (FreeTubeApp#3963)
  Bump marked from 7.0.4 to 7.0.5 (FreeTubeApp#3961)
  Bump @babel/core from 7.22.10 to 7.22.11 (FreeTubeApp#3959)
  Make certain controls non-selectable / non-draggable (FreeTubeApp#3947)
  Add updated video resolution to auto selector (FreeTubeApp#3935)
  Fix Save icon blocking issue (FreeTubeApp#3951)
  * Update URL parser to recognize youtube.com/live/xxxxxxx (FreeTubeApp#3930)
  Translated using Weblate (Czech)
  Translated using Weblate (Portuguese)
  Translated using Weblate (Polish)
  Translated using Weblate (Arabic)
  ...

# Conflicts:
#	src/renderer/main.js
#	src/renderer/scss-partials/_ft-list-item.scss
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Aug 29, 2023
@PikachuEXE
Merge branch 'feature/playlist-2023-05' into custom-builds/current
aaf5f03 
* feature/playlist-2023-05: (77 commits)
  * Update add to playlist prompt to add sort options
  ! Fix user playlist view incorrectly sorted playlists by latest updated first when filtered
  ! Fix add to play prompt max width
  * Update sorting options labels
  Make constants imported from @fortawesome/free-solid-svg-icons ordered by name again (FreeTubeApp#3958)
  Replace labeler workflow with GH labeler (FreeTubeApp#3966)
  ! Fix unable to visit previous video when playing first video in a playlist
  * Update upcoming video to allow saving in playlist
  * Show filtering input & sorting element when no. of playlist > 1
  Fix HTML styling (e.g., hashtag links) showing up as raw HTML in descriptions (FreeTubeApp#3946)
  Make certain controls non-selectable / non-draggable - Part II (FreeTubeApp#3957)
  Translated using Weblate (German)
  Show video list buttons on hover or focus (FreeTubeApp#3954)
  Bump @babel/eslint-parser from 7.22.10 to 7.22.11 (FreeTubeApp#3962)
  Bump youtubei.js from 6.0.0 to 6.1.0 (FreeTubeApp#3965)
  Bump eslint-plugin-n from 16.0.1 to 16.0.2 (FreeTubeApp#3964)
  Bump eslint from 8.47.0 to 8.48.0 (FreeTubeApp#3963)
  Bump marked from 7.0.4 to 7.0.5 (FreeTubeApp#3961)
  Bump @babel/core from 7.22.10 to 7.22.11 (FreeTubeApp#3959)
  Make certain controls non-selectable / non-draggable (FreeTubeApp#3947)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LQ2-apostrophe LQ2-apostrophe LQ2-apostrophe left review comments

@PikachuEXE PikachuEXE PikachuEXE approved these changes

@absidue absidue absidue approved these changes

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc approved these changes

@ChunkyProgrammer ChunkyProgrammer Awaiting requested review from ChunkyProgrammer

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

Successfully merging this pull request may close these issues.
7 participants
@kommunarr @PikachuEXE @efb4f5ff-1298-471a-8973-3d47447115dc @LQ2-apostrophe @absidue @ChunkyProgrammer @FreeTubeBot