Skip to content

feat(files): support nested actions - #41010

Merged
skjnldsv merged 4 commits into
masterfrom
feat/nested-actins
Nov 8, 2023
Merged

feat(files): support nested actions#41010
skjnldsv merged 4 commits into
masterfrom
feat/nested-actins

Conversation

@skjnldsv

@skjnldsvskjnldsv commented Oct 20, 2023•
edited
Loading

Copy link
Copy Markdown
Member

@skjnldsvskjnldsv self-assigned this Oct 20, 2023
@skjnldsvskjnldsv mentioned this pull request Oct 20, 2023
Merged
@skjnldsv
skjnldsv requested a review from szaimenOctober 20, 2023 10:40
skjnldsv
skjnldsv commented Oct 20, 2023
View reviewed changes
Comment threadapps/files/src/init.ts Outdated
@skjnldsv
skjnldsv marked this pull request as draft October 20, 2023 10:40
@szaimen

This comment was marked as resolved.

Sign in to view
@skjnldsv

This comment was marked as resolved.

Sign in to view
@szaimen

This comment was marked as resolved.

Sign in to view
Pytal

This comment was marked as resolved.

Sign in to view

@skjnldsv

This comment was marked as resolved.

Sign in to view
marcoambrosini

This comment was marked as off-topic.

Sign in to view

@skjnldsv

This comment was marked as resolved.

Sign in to view
@marcoambrosinimarcoambrosini mentioned this pull request Oct 25, 2023
Closed
@nimishavijay

This comment was marked as resolved.

Sign in to view
@skjnldsv

This comment was marked as resolved.

Sign in to view
@Pytal

This comment was marked as resolved.

Sign in to view
@solracsfsolracsf added this to the Nextcloud 28 milestone Oct 27, 2023
This was referenced Oct 31, 2023
Merged
Merged
jancborchardt

This comment was marked as off-topic.

Sign in to view

@jancborchardt

This comment was marked as resolved.

Sign in to view
@skjnldsv
skjnldsv marked this pull request as ready for review November 3, 2023 17:56
@skjnldsv

skjnldsv commented Nov 3, 2023•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

@Pytal please review the logic!

Especially the condition on where a reminder is shown or not, as I wasn't part of the original discussions! I used your services, so I think it should be good.
But here are a few questions to be safe

  1. Is it normal that depending on the time of day, only some options are visible?
  2. Is this feature enabled for all nodes, no matter what they are and what permissions they have? (folder/files, etc etc)

@skjnldsv
skjnldsv requested review from Pytal and artongeNovember 3, 2023 18:35
@Pytal

Pytal commented Nov 4, 2023

Copy link
Copy Markdown
Member
  1. Is it normal that depending on the time of day, only some options are visible?

Yes this is by design

  1. Is this feature enabled for all nodes, no matter what they are and what permissions they have? (folder/files, etc etc)

Yes, if you can see (read) the file then you should be able to set a reminder on it

Pytal
Pytal reviewed Nov 4, 2023
View reviewed changes
Comment threadapps/files/src/components/FileEntry/FileEntryActions.vue Outdated
Comment threadapps/files/src/components/FileEntry/FileEntryActions.vue Outdated
Comment threadapps/files_reminders/src/actions/setReminderSuggestionActions.scss Outdated
Comment threadapps/files_reminders/src/actions/setReminderSuggestionActions.ts
Comment threadapps/files_reminders/src/actions/setReminderSuggestionActions.ts Outdated
Comment threadapps/files_reminders/src/actions/setReminderSuggestionActions.ts Outdated
Comment threadapps/files_reminders/src/components/SetCustomReminderModal.vue Outdated
Comment threadapps/files_reminders/src/services/customPicker.ts Outdated
@blizzzblizzz mentioned this pull request Nov 6, 2023
Merged
@skjnldsvskjnldsv mentioned this pull request Nov 7, 2023
Open
@skjnldsv
skjnldsv requested a review from PytalNovember 7, 2023 10:41
susnux
susnux requested changes Nov 7, 2023
View reviewed changes

@susnuxsusnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The focus handling does not work properly:
If you click (keyboard) on "set reminder" the submenu opens but the focus is not set on the submenu's first element.

Also if you click the back button the first submenu is reopend, but the focus is not on the "set reminder" entry.

In both cases the focus is on some random non visible element.

Pytal
Pytal reviewed Nov 7, 2023
View reviewed changes
Comment threadapps/files_reminders/src/actions/setReminderSuggestionActions.ts
}

// Generate the default preset actions
export const actions = [laterToday, tomorrow, thisWeekend, nextWeek]

@PytalPytalNov 7, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since the presence of the action should be evaluated ideally on click instead of on script execution I think the FileAction enabled callback needs to be set to something like () => Boolean(getDateTime(option.dateTimePreset)) to be more reactive

@skjnldsvskjnldsvNov 7, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Can be improved later

susnux
susnux approved these changes Nov 7, 2023
View reviewed changes

@susnuxsusnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🐘 a11y in follow up?

@skjnldsv
feat(files): support nested actions
72ffd49
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv
chore(files_reminders): upgrade to 28 APIs
cf6c921
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv
chore(jest): update snapshot after component update
50417bc
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv
fix(cypress): formatFileSize change revert from files library
32c1aeb
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv
skjnldsv merged commit 069eb07 into masterNov 8, 2023
@skjnldsv
skjnldsv deleted the feat/nested-actins branch November 8, 2023 07:53
This was referenced Nov 8, 2023
Merged
Closed
@max-nextcloudmax-nextcloud mentioned this pull request Dec 15, 2023
Closed
8 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jancborchardtjancborchardtjancborchardt left review comments
@marcoambrosinimarcoambrosinimarcoambrosini left review comments
@susnuxsusnuxsusnux approved these changes
@PytalPytalPytal approved these changes
@szaimenszaimenAwaiting requested review from szaimen
@artongeartongeAwaiting requested review from artonge
@JuliaKirschenheuterJuliaKirschenheuterAwaiting requested review from JuliaKirschenheuter
@ShGKmeShGKmeAwaiting requested review from ShGKme

Assignees

@skjnldsvskjnldsv

Labels

3. to reviewWaiting for reviewsenhancementfeature: files

Projects

Files to vue
Status: Done

Milestone

Nextcloud 28

Development

Successfully merging this pull request may close these issues.

Add API for FileAction submenu actions

8 participants

@skjnldsv@szaimen@nimishavijay@Pytal@jancborchardt@susnux@marcoambrosini@solracsf