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 - kv v2 versions page #5563

Merged
merged 7 commits into from
Oct 19, 2018
Merged

UI - kv v2 versions page #5563

merged 7 commits into from
Oct 19, 2018

Conversation

meirish
Copy link
Contributor

@meirish meirish commented Oct 19, 2018

Version link from the list page:
screen shot 2018-10-19 at 4 09 51 pm

Version link from the show page:
screen shot 2018-10-19 at 4 10 46 pm

New Version history page:
screen shot 2018-10-19 at 4 10 24 pm

.adapterFor('secret-v2-version')
.v2DeleteOperation(this.store, this.version.id, deleteType);
},
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of this was in secret-edit and is now moved to its own component so that we could use it on the show page, and now on the version history page.

@meirish meirish requested review from a team October 19, 2018 21:15
@joshuaogle
Copy link
Contributor

👍 on the template side and looks good in action. I'll let someone else approve from the JS side though

(can we remove the is-wide from the Secrets List page (first screenshot)? It fits great without it)

@meirish
Copy link
Contributor Author

meirish commented Oct 19, 2018

Oh yep! Not sure why that got added there.... Thanks for checking it out!

@andaley andaley self-assigned this Oct 19, 2018
</div>
</div>
<div class="level-left is-flex-1">
{{#link-to params=linkParams class="has-text-weight-semibold has-text-black is-display-flex is-flex-1 is-no-underline"}}
Copy link
Contributor

Choose a reason for hiding this comment

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

whew, that's quite the class list. 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😬 Yeah - hoping to move away from this - or at least hide it under the rug with more components

@tagName="button"
>
{{#if useDefaultTrigger}}
<ICon aria-label="More options" @glyph="more" @size="16" @class="has-text-black auto-width" />
Copy link
Contributor

Choose a reason for hiding this comment

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

ah, funny. is the component named ICon because ember forces each component to have a dash in the name? 🤣

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lol exactly - the new angle bracket syntax doesn't have that limitation, but we'll have to convert it everywhere at once.

andaley
andaley previously approved these changes Oct 19, 2018
@meirish meirish merged commit a58745b into master Oct 19, 2018
@meirish meirish deleted the ui-kv-v2-versions branch October 19, 2018 22:25
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