Skip to content

feat: Show lock status next to unlock button #587

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

Merged
merged 3 commits into from
Apr 22, 2025

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Mar 14, 2025

We can make use of the enabled check to hide the menu entry, which was commented out but seems to work nowadays.

That way we now can properly show a action menu entry as info about the lock state only if the file is locked.

Unlocked Locked
List view Screenshot 2025-03-18 at 22 34 55 Screenshot 2025-03-18 at 22 34 42
Grid view Screenshot 2025-03-18 at 22 35 10 Screenshot 2025-03-18 at 22 35 17

@juliusknorr
Copy link
Member Author

@susnux I've been trying to work with this, however I noticed that when in grid view the action uses the title instead of the display name for the regular file action. Is this expected?

@susnux
Copy link
Contributor

susnux commented Mar 18, 2025

Is this expected?

Yes it was implemented that way by @skjnldsv
E.g. inline sharing action has displayname "shared" and title "shared by XYZ" then the grid view action shill show "shared by XYZ" as the text in the NcActions.

@juliusknorr juliusknorr force-pushed the fix/lock-indicator-enabled branch from b14a470 to e50814a Compare March 18, 2025 21:32
@juliusknorr
Copy link
Member Author

Ok, rather unexpected but i think i found a feasible way now with adding an extra action to display the state. However it still uses the css to hide the inline action as I found it still valuable to have the state written out in both list and grid view.

@juliusknorr juliusknorr marked this pull request as ready for review March 18, 2025 21:40
@juliusknorr juliusknorr changed the title fix: Properly disable inline action if not available feat: Show lock status next to unlock button Mar 18, 2025
Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

I think it would be more appropriate to use ncactioncaption for this, otherwise this info text just looks to similar to other action buttons.

@susnux
Copy link
Contributor

susnux commented Mar 19, 2025

I think it would be more appropriate to use ncactioncaption for this, otherwise this info text just looks to similar to other action buttons.

Thats not possible currently. You can only register actions (buttons) not text.

@marcoambrosini
Copy link
Member

Got it. Can we at least make the button disabled (no hover, no cursor: pointer, not tabbable)?

@susnux
Copy link
Contributor

susnux commented Mar 19, 2025

Got it. Can we at least make the button disabled (no hover, no cursor: pointer, not tabbable)?

Also this is not possible - if the action is disabled it is not rendered at all.

I think if we want to have those status information we should do it as text and not introduce disabled buttons - those are an a11y nightmare. For now this way is probably the only feasible but we can of course implement improvements on the files app like you suggested @marcoambrosini :)

@juliusknorr juliusknorr force-pushed the fix/lock-indicator-enabled branch from e50814a to 8ad2156 Compare April 3, 2025 06:32
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Code looks sane, and thanks for the tests ❤️

@juliusknorr juliusknorr force-pushed the fix/lock-indicator-enabled branch from 8ad2156 to fd9fc1c Compare April 22, 2025 07:01
@skjnldsv skjnldsv merged commit 94a7252 into main Apr 22, 2025
44 of 47 checks passed
@skjnldsv skjnldsv deleted the fix/lock-indicator-enabled branch April 22, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants