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

UI: Namespace area fixes #19799

Merged
merged 5 commits into from
Apr 5, 2023
Merged

UI: Namespace area fixes #19799

merged 5 commits into from
Apr 5, 2023

Conversation

hashishaw
Copy link
Collaborator

This PR fixes a couple issues with the namespaces area of the UI:

Goes to the wrong link if namespace has dots
wrong link when namespace with dots

Shows blue info banner from unexpected param
image

After

namespace-fixes

@hashishaw hashishaw added this to the 1.14 milestone Mar 28, 2023
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for turning into class based!

@@ -4,21 +4,21 @@
*/

import Model, { attr } from '@ember-data/model';
import { computed } from '@ember/object';
import { expandAttributeMeta } from 'vault/utils/field-to-attrs';
import { withExpandedAttributes } from 'vault/decorators/model-expanded-attributes';
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

@@ -35,7 +35,7 @@
{{#let (concat this.currentNamespace (if this.currentNamespace "/") list.item.id) as |targetNamespace|}}
{{#if (includes targetNamespace this.accessibleNamespaces)}}
<li class="action">
<NamespaceLink @targetNamespace={{targetNamespace}} @class="is-block">
<NamespaceLink @targetNamespace={{targetNamespace}} @unparsed={{true}} @class="is-block">
Copy link
Contributor

Choose a reason for hiding this comment

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

would you mind adding documentation to the NamespaceLink component? Add unparsed parma, etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add a comment about unparsed, but there's a lot in there that I have no idea what's going on 😬

Copy link
Contributor

@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

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

Looks good to me! One request, but non-blocking.

@hashishaw hashishaw merged commit 44cf5e9 into main Apr 5, 2023
@hashishaw hashishaw deleted the ui/namespace-fixes branch April 5, 2023 15:54
hashishaw added a commit that referenced this pull request Apr 6, 2023
hashishaw added a commit that referenced this pull request Apr 6, 2023
hashishaw added a commit that referenced this pull request Apr 6, 2023
* UI: Namespace area fixes (#19799)

* revert model changes due to unavailable decorator
hashishaw added a commit that referenced this pull request Apr 7, 2023
* UI: Namespace area fixes (#19799)

* revert model changes due to unavailable decorator
hashishaw added a commit that referenced this pull request Apr 7, 2023
hashishaw added a commit that referenced this pull request Apr 7, 2023
* UI: Namespace area fixes (#19799)

* Revert model changes due to unavailable decorator
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.

None yet

2 participants