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 Action from CheckboxBlankCircle and add more description text #4045

Conversation

JuliaKirschenheuter
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter commented Mar 11, 2022

@codecov
Copy link

codecov bot commented Mar 11, 2022

Codecov Report

Merging #4045 (6746ddc) into main (e6b4f9f) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

❗ Current head 6746ddc differs from pull request most recent head f1686f2. Consider uploading reports for the commit f1686f2 to get more accurate results

@@             Coverage Diff              @@
##               main    #4045      +/-   ##
============================================
- Coverage     29.39%   29.36%   -0.04%     
  Complexity      330      330              
============================================
  Files           221      221              
  Lines          7712     7721       +9     
  Branches       1022     1026       +4     
============================================
  Hits           2267     2267              
- Misses         5445     5454       +9     
Flag Coverage Δ
javascript 20.47% <0.00%> (-0.03%) ⬇️
php 68.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ts/AppNavigation/CalendarList/CalendarListItem.vue 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6b4f9f...f1686f2. Read the comment docs.

Copy link
Member

@raimund-schluessler raimund-schluessler left a comment

Choose a reason for hiding this comment

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

I have to admit, I don't really understand why this shouldn't be an Action, but this anyway needs changes to work. Please see my comments.

Copy link
Member

@raimund-schluessler raimund-schluessler left a comment

Choose a reason for hiding this comment

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

Looks good code wise.

@JuliaKirschenheuter
Copy link
Contributor Author

I have to admit, I don't really understand why this shouldn't be an Action, but this anyway needs changes to work. Please see my comments.

After review with @jancborchardt we have decided not to to have a same action on two different components: AppNavigationItem and CheckboxBlankCircle. User click on AppNavigationItem -> toggle will be triggered, and it is enough.

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/3955-Calendar_visibility_toggle_(entry_itself)_is_not_focusable_via_keyboard branch from efa8a35 to 3efa4af Compare March 14, 2022 15:22
@JuliaKirschenheuter JuliaKirschenheuter marked this pull request as ready for review March 14, 2022 15:23
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/3955-Calendar_visibility_toggle_(entry_itself)_is_not_focusable_via_keyboard branch from 3efa4af to 0872c8c Compare March 18, 2022 13:15
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/3955-Calendar_visibility_toggle_(entry_itself)_is_not_focusable_via_keyboard branch from 0872c8c to 03be8db Compare March 21, 2022 11:22
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/3955-Calendar_visibility_toggle_(entry_itself)_is_not_focusable_via_keyboard branch from 03be8db to 307c5f7 Compare March 21, 2022 13:20
@ChristophWurst ChristophWurst added the blocked This ticket or PR is blocked by another one label Mar 21, 2022
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/3955-Calendar_visibility_toggle_(entry_itself)_is_not_focusable_via_keyboard branch from 307c5f7 to ed90f15 Compare March 21, 2022 15:48
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

👍 otherwise

@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/3955-Calendar_visibility_toggle_(entry_itself)_is_not_focusable_via_keyboard branch from ed90f15 to bccea49 Compare March 22, 2022 07:01
@ChristophWurst ChristophWurst marked this pull request as draft March 22, 2022 08:25
@ChristophWurst
Copy link
Member

Marking as draft to prevent an accidental merge before main ships @nextcloud/vue with nextcloud-libraries/nextcloud-vue#2548.

@JuliaKirschenheuter
Copy link
Contributor Author

Marking as draft to prevent an accidental merge before main ships @nextcloud/vue with nextcloud/nextcloud-vue#2548.

i think we can merge it: https://github.com/nextcloud/nextcloud-vue/releases/tag/v4.4.0

@JuliaKirschenheuter JuliaKirschenheuter marked this pull request as ready for review June 20, 2022 07:27
@ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed blocked This ticket or PR is blocked by another one 3. to review Waiting for reviews labels Jun 27, 2022
Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
@JuliaKirschenheuter JuliaKirschenheuter force-pushed the fix/3955-Calendar_visibility_toggle_(entry_itself)_is_not_focusable_via_keyboard branch from 6746ddc to f1686f2 Compare June 28, 2022 15:09
@JuliaKirschenheuter JuliaKirschenheuter merged commit e2a2707 into main Jun 28, 2022
@JuliaKirschenheuter JuliaKirschenheuter deleted the fix/3955-Calendar_visibility_toggle_(entry_itself)_is_not_focusable_via_keyboard branch June 28, 2022 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calendar visibility toggle (entry itself) is not focusable via keyboard
4 participants