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

feat: introduce mode config option #9818

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

LukasHirt
Copy link
Collaborator

@LukasHirt LukasHirt commented Oct 16, 2023

Description

Introduce new config option to assert in which mode the UI should be rendered. There are two ways to set the mode, either by adding it into the options object in the config or via mode query param. The config option takes precedence when both are defined. To showcase the functionality of the config, hide elements in the embed mode in the top bar component.

Related Issue

Motivation and Context

This is needed to have functioning embed mode. Providing both config and query approaches makes us future proof. Same applies to having the option as mode?: string instead of e.g. embedded?: boolean so that any other mode could be introduced in the future...

How Has This Been Tested?

  • test environment: local dev env
  • test case 1: open https://localhost:9200?mode=embed
  • test case 2: set options.mode to embed in config.json - does not work because of custom config not working (but unit tests are passing)

Screenshots (if appropriate):

localhost_9200__mode=embed

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 added
  • Acceptance tests added
  • Documentation ticket raised:

@update-docs
Copy link

update-docs bot commented Oct 16, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@LukasHirt LukasHirt marked this pull request as draft October 16, 2023 18:33
@LukasHirt LukasHirt marked this pull request as ready for review October 16, 2023 19:31
@LukasHirt LukasHirt requested a review from dschmidt October 16, 2023 19:32
@sonarcloud
Copy link

sonarcloud bot commented Oct 16, 2023

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

81.8% 81.8% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@JammingBen JammingBen left a comment

Choose a reason for hiding this comment

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

test case 2: set options.mode to embed in config.json

Does this work for you without setting the query param? Normally it shouldn't because the config is still missing in oCIS 🤔

@LukasHirt
Copy link
Collaborator Author

Does this work for you without setting the query param? Normally it shouldn't because the config is still missing in oCIS 🤔

Strange... I tried it once and it worked. I must have screwed up somehow my local env. Trying it again does not work indeed.

Anyway, I guess we can merge this still? The unit test for the config value is passing so once config is working in oCIS - it should work 🤷

@JammingBen JammingBen merged commit 411fc98 into owncloud:master Oct 18, 2023
2 checks passed
@LukasHirt LukasHirt deleted the feat/mode-config branch October 18, 2023 15:13
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.

3 participants