Skip to content

Conversation

lukasdotcom
Copy link
Member

No description provided.

Signed-off-by: Lukas Schaefer <lukas@lschaefer.xyz>
Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

  • The v-tooltips do not work. They can be replaced by a simple title attr
  • [Unrelated] You should change the app version number when making a PR that contains DB migrations. I forgot to tell you in the PR adding the unapprove_when_modified column. Let's do it here. You can bump the 2nd digit.
  • [Unrelated] Line 286 of AdminSettings.vue, the unapproveWhenModified value should be a boolean. This breaks the creation of a new rule in the UI. Vue complains about the NcCheckboxRadioSwitch model-value prop type and crashes.

Otherwise, all good 👍

@julien-nc julien-nc self-requested a review June 25, 2025 14:57
@lukasdotcom
Copy link
Member Author

  • The v-tooltips do not work. They can be replaced by a simple title attr

Also those tooltips should probably not be there when you aren't the admin.

Signed-off-by: Lukas Schaefer <lukas@lschaefer.xyz>
Signed-off-by: Lukas Schaefer <lukas@lschaefer.xyz>
Signed-off-by: Lukas Schaefer <lukas@lschaefer.xyz>
@lukasdotcom lukasdotcom requested a review from marcelklehr as a code owner June 25, 2025 15:25
@julien-nc
Copy link
Member

Also those tooltips should probably not be there when you aren't the admin.

Even the + buttons should be hidden on the same condition as the "create new hidden tag" input.

Signed-off-by: Lukas Schaefer <lukas@lschaefer.xyz>
@lukasdotcom
Copy link
Member Author

lukasdotcom commented Jun 25, 2025

Also those tooltips should probably not be there when you aren't the admin.

Even the + buttons should be hidden on the same condition as the "create new hidden tag" input.

I had removed the buttons and the tooltips my comment just didn't mention that 👍

That should now be everything you mentioned.

@lukasdotcom lukasdotcom requested a review from julien-nc June 25, 2025 15:33
Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

👍

@lukasdotcom lukasdotcom merged commit e97c64c into main Jun 25, 2025
39 checks passed
@lukasdotcom lukasdotcom deleted the vue3 branch June 25, 2025 16:10
@lukasdotcom lukasdotcom mentioned this pull request Jun 26, 2025
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.

2 participants