-
Notifications
You must be signed in to change notification settings - Fork 293
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
Fixes #36576 - Add Activation Key details top bar #10637
Conversation
Issues: #36576 |
Can one of the admins verify this patch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Haven't tested but leaving few nitpicks..
PS: Audit the strings in the components for translations: __('Will be translated')
.
webpack/scenes/ActivationKeys/Details/components/DeleteModal.js
Outdated
Show resolved
Hide resolved
webpack/scenes/ActivationKeys/Details/components/DeleteModal.js
Outdated
Show resolved
Hide resolved
[test katello] |
…strings, fix host limit issue with edit button, make akId required, import useState in delete menu
@sjha4 updated. Thanks |
[test katello] |
When editing max_hosts in edit modal, I am unable to reduce the value. I am seeing this.
|
Also seeing some console errors as soon as I open edit modal:
|
Able to edit/delete AK. Able to navigate to new AK page and old page with new page action. The host_limit correctly displays on the UI and the breadcrumb redirects to the AK index page. ✅ The page looks good and works great other than the couple of issues above. Nice work! |
React CI lint failures are related.. |
[test katello] |
9409eb5
to
4784d5a
Compare
[test katello] |
Couple of notes:
|
@sjha4 updated |
[test katello] |
[test katello] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor nitpicks.. Looks ready to go otherwise.. 🎉
107b177
to
84a3110
Compare
[test katello] |
One thing I just noticed is AK create/update/delete etc are tasks and sometimes they can take longer than modal closing. I filed an issue to handle that with some sort of loading state or whatever UX suggests. https://projects.theforeman.org/issues/36635 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack. Nice work @trev-allison03 ! 🎉
What are the changes introduced in this pull request?
Considerations taken when implementing this change?
-used window.location.replace instead of because redirect to an angularJS page did not work
What are the testing steps for this pull request?