Fixes #25618 - Use PF3-React Delete Confirmation Dialog #6301

boaz0 · 2018-12-04T13:05:26Z

No description provided.

theforeman-bot · 2018-12-04T13:05:35Z

Issues: #25618

amirfefer

Thanks @boaz1337 !
where do you think to use this component beside storybook?

webpack/assets/javascripts/react_app/components/DeleteMessageDialog/index.js

boaz0 · 2018-12-05T11:38:42Z

Hi @amirfefer
Thanks for the review although this is still work in progress.
This will be used in the Hardware Models page replacing the old alert message when you delete a model.

glekner · 2018-12-06T10:38:45Z

I guess we started working on the same issue together :)

I'll leave it to you, here is my WIP if it can help

...ck/assets/javascripts/react_app/components/DeleteMessageDialog/deleteConfirmationDialog.scss

ohadlevy · 2018-12-30T19:12:43Z

@glekner can you review please? thanks

app/views/models/index.html.erb

webpack/assets/javascripts/react_app/components/DeleteMessageDialog/DeleteConfirmationDialog.js

glekner

Thanks @boaz1337! Looks great

glekner · 2018-12-31T13:58:19Z

...ck/assets/javascripts/react_app/components/DeleteMessageDialog/DeleteMessageDialogActions.js

+  dispatch({ type: DELETE_MESSAGE_DIALOG_REQUEST });
+  API.delete(`/api/${controller}/${id}`)
+    .then(resp => {
+      window.Turbolinks.visit(`/${controller}`);


Can we use something like onDeleteSucceed, onDeleteError as props for this action?

webpack/assets/javascripts/react_app/components/DeleteMessageDialog/DeleteConfirmationDialog.js

coveralls · 2019-01-03T12:53:47Z

Coverage decreased (-0.2%) to 69.413% when pulling 1e46819 on boaz1337:fixes_25618 into 71ae1b6 on theforeman:develop.

coveralls · 2019-01-03T12:53:48Z

Coverage decreased (-0.2%) to 69.401% when pulling 69ad1c8 on boaz1337:fixes_25618 into 71ae1b6 on theforeman:develop.

ohadlevy · 2019-01-15T11:46:52Z

@glekner can you have another round of review? what would it take to implement it across all tables?

app/helpers/delete_message_dialog_helper.rb

app/views/models/index.html.erb

webpack/assets/javascripts/react_app/components/DeleteMessageDialog/DeleteConfirmationDialog.js

ohadlevy · 2019-01-24T13:59:12Z

webpack/assets/javascripts/react_app/components/DeleteMessageDialog/DeleteConfirmationDialog.js

+          href={`/${controller}/${id}-${name}`}
+          data-method="delete"
+        >
+          <Spinner loading={processing} inline size="xs" />


when would processing be set to true?

from the moment clicking on the delete it is set to true and till the page is refreshed.

webpack/assets/javascripts/react_app/components/DeleteMessageDialog/DeleteConfirmationDialog.js

Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com> Co-authored-by: Gilad Lekner <gilad215@gmail.com>

ohadlevy · 2019-02-13T13:26:49Z

@boaz0 can you have a quick look on the test failures?

ohadlevy · 2019-04-07T14:04:18Z

@glekner any chance you can follow up this work? since the hw model page already is a table in react, perhaps we can start with a simple follow up to add that there, and then use the rails helpers etc in this PR to make it available to other pages too?

Ron-Lavi

Thanks @boaz0 👍
Didn't test it yet but it looks good !
Left some inline comments :)

Ron-Lavi · 2019-04-07T16:35:14Z

app/views/models/index.html.erb

        </td>
      </tr>
    <% end %>
  </tbody>
 </table>
 <%= will_paginate_with_info @models %>
+<div id="delete-modal"></div>


could you create the div in the helper instead?
for example:

container_class_name = "continaer-#{props[:id]}" content_tag(:div, nil, :class => container_class_name) + mount_react_component( 'ComponentWrapper', ".#{container_class_name}", { component: 'DeleteMessageDialog', componentProps: props }.to_json )

Ron-Lavi · 2019-04-07T16:41:52Z

...ck/assets/javascripts/react_app/components/DeleteMessageDialog/DeleteMessageDialogReducer.js

+      });
+    case DELETE_MESSAGE_DIALOG_OPEN:
+      return state.merge({
+        ...action.payload,


can you destruct it so we will see here what is inside the payload: name, url

Ron-Lavi · 2019-04-07T16:43:26Z

webpack/assets/javascripts/react_app/components/DeleteMessageDialog/index.js

+import reducer from './DeleteMessageDialogReducer';
+
+const mapStateToProps = state => ({
+  show: state.deleteDialog.show,


can you use selectors ? :)

xprazak2 · 2019-08-02T12:25:09Z

@boaz0, could you rebase please?

theforeman-bot · 2019-08-21T08:16:50Z

@boaz0, this pull request is currently not mergeable. Please rebase against the develop branch and push again.

If you have a remote called 'upstream' that points to this repository, you can do this by running:

    $ git pull --rebase upstream develop

This message was auto-generated by Foreman's prprocessor

tbrisker · 2019-08-28T14:25:51Z

Thanks @boaz0 ! This PR has been inactive for a very long time and is not currently mergeable. I'll go ahead and close it for now, feel free to reopen if you wish to get back to it.

boaz0 · 2019-08-28T16:52:54Z

Yeah, I was trying to find a free time to work on it but I guess this is not going to happen soon. Anyone who wishes to take this is welcome. 😀

//cc @amirfefer @sharvit @theforeman/ui-ux

Sorry

theforeman-bot added Needs testing Not yet reviewed UI labels Dec 4, 2018

amirfefer suggested changes Dec 5, 2018

View reviewed changes

theforeman-bot added Waiting on contributor and removed Not yet reviewed labels Dec 5, 2018

theforeman-bot added Needs re-review and removed Waiting on contributor labels Dec 25, 2018

houndci-bot reviewed Dec 25, 2018

View reviewed changes

...ck/assets/javascripts/react_app/components/DeleteMessageDialog/deleteConfirmationDialog.scss Show resolved Hide resolved

...ck/assets/javascripts/react_app/components/DeleteMessageDialog/deleteConfirmationDialog.scss Show resolved Hide resolved

boaz0 changed the title ~~[WIP] Fixes #25618 - Use PF3-React Delete Confirmation Dialog~~ Fixes #25618 - Use PF3-React Delete Confirmation Dialog Dec 27, 2018

ohadlevy reviewed Dec 30, 2018

View reviewed changes

app/views/models/index.html.erb Show resolved Hide resolved

ohadlevy reviewed Dec 30, 2018

View reviewed changes

webpack/assets/javascripts/react_app/components/DeleteMessageDialog/DeleteConfirmationDialog.js Outdated Show resolved Hide resolved

glekner reviewed Dec 31, 2018

View reviewed changes

ohadlevy suggested changes Jan 24, 2019

View reviewed changes

theforeman-bot added Waiting on contributor and removed Needs re-review labels Jan 24, 2019

Fixes #25618 - Use PF3-React Delete Confirmation Dialog

205cafc

Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com> Co-authored-by: Gilad Lekner <gilad215@gmail.com>

theforeman-bot added Needs re-review and removed Waiting on contributor labels Jan 27, 2019

ohadlevy mentioned this pull request Jan 31, 2019

Fixes #25888 - move components from Katello to Foreman #6441

Merged

Ron-Lavi reviewed Apr 7, 2019

View reviewed changes

tbrisker assigned xprazak2 Aug 21, 2019

theforeman-bot added Waiting on contributor and removed Needs testing Needs re-review labels Aug 21, 2019

tbrisker closed this Aug 28, 2019

boaz0 deleted the fixes_25618 branch August 28, 2019 16:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #25618 - Use PF3-React Delete Confirmation Dialog #6301

Fixes #25618 - Use PF3-React Delete Confirmation Dialog #6301

boaz0 commented Dec 4, 2018

theforeman-bot commented Dec 4, 2018

amirfefer left a comment

boaz0 commented Dec 5, 2018

glekner commented Dec 6, 2018

ohadlevy commented Dec 30, 2018

glekner left a comment

glekner Dec 31, 2018

coveralls commented Jan 3, 2019

coveralls commented Jan 3, 2019 •

edited

Loading

ohadlevy commented Jan 15, 2019

ohadlevy Jan 24, 2019

boaz0 Jan 27, 2019 •

edited

Loading

ohadlevy commented Feb 13, 2019

ohadlevy commented Apr 7, 2019

Ron-Lavi left a comment

Ron-Lavi Apr 7, 2019

Ron-Lavi Apr 7, 2019

Ron-Lavi Apr 7, 2019

xprazak2 commented Aug 2, 2019

theforeman-bot commented Aug 21, 2019

tbrisker commented Aug 28, 2019

boaz0 commented Aug 28, 2019

Fixes #25618 - Use PF3-React Delete Confirmation Dialog #6301

Fixes #25618 - Use PF3-React Delete Confirmation Dialog #6301

Conversation

boaz0 commented Dec 4, 2018

theforeman-bot commented Dec 4, 2018

amirfefer left a comment

Choose a reason for hiding this comment

boaz0 commented Dec 5, 2018

glekner commented Dec 6, 2018

ohadlevy commented Dec 30, 2018

glekner left a comment

Choose a reason for hiding this comment

glekner Dec 31, 2018

Choose a reason for hiding this comment

coveralls commented Jan 3, 2019

coveralls commented Jan 3, 2019 • edited Loading

ohadlevy commented Jan 15, 2019

ohadlevy Jan 24, 2019

Choose a reason for hiding this comment

boaz0 Jan 27, 2019 • edited Loading

Choose a reason for hiding this comment

ohadlevy commented Feb 13, 2019

ohadlevy commented Apr 7, 2019

Ron-Lavi left a comment

Choose a reason for hiding this comment

Ron-Lavi Apr 7, 2019

Choose a reason for hiding this comment

Ron-Lavi Apr 7, 2019

Choose a reason for hiding this comment

Ron-Lavi Apr 7, 2019

Choose a reason for hiding this comment

xprazak2 commented Aug 2, 2019

theforeman-bot commented Aug 21, 2019

tbrisker commented Aug 28, 2019

boaz0 commented Aug 28, 2019

coveralls commented Jan 3, 2019 •

edited

Loading

boaz0 Jan 27, 2019 •

edited

Loading