Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

Enhancement: Give OcModal the option to use OcContextualHelper #2325

Merged
merged 2 commits into from
Sep 30, 2022

Conversation

lookacat
Copy link
Contributor

@lookacat lookacat commented Sep 19, 2022

Description

See owncloud/web#6892 (comment)

Related Issue

owncloud/web#6892

Motivation and Context

Why new props instead of using slot: So we can use it via code without markup

Screenshots (if appropriate):

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests updated
  • Acceptance tests added
  • Documentation added/updated

@lookacat lookacat changed the title Add OcContextualHelper to OcModal Enhancement: Give OcModal the option to use OcContextualHelper Sep 19, 2022
@sonarcloud
Copy link

sonarcloud bot commented Sep 19, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@AlexAndBear
Copy link

I am not quite sure if we should do this.
It would be better to use a slot for the message and implement the contextualHelper there.

@lookacat
Copy link
Contributor Author

The slot method is not suitable because we create the modal via code, we have no option there to dynamically provide a template

@AlexAndBear
Copy link

AlexAndBear commented Sep 30, 2022

As far as I know it will be rendered in the code via vue template anyways.So it should be quite possible to pass a new variable for the ct helper in web.

It's just not a good approach at the end to add a lot of components we have to the modal it self.

@AlexAndBear
Copy link

Rechecked, the slot won't be used in the app.vue, so I agree that's the easiest solution here.

@lookacat lookacat merged commit addaf3d into master Sep 30, 2022
@delete-merged-branch delete-merged-branch bot deleted the add-contextual-helper-to-modal branch September 30, 2022 12:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants