Skip to content

Conversation

@PrakashDurlabhji
Copy link
Contributor

No description provided.

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

@PrakashDurlabhji

The current fix doesn't work for me.

image

Confirmation dialog should be opened under the status select. And confirmation dialog should "push" content bellow when opened, not shown on the top. See task description #3206 (comment)

Let me know if you would like to try to complete it, or open this issue for pick up.

@PrakashDurlabhji
Copy link
Contributor Author

@maxceem I will complete it

@PrakashDurlabhji
Copy link
Contributor Author

@maxceem PR updated and it will be shown below dropdown "Project is active".

Kindly provide feedback if any because current behaviour is that when "cancelled" is clicked then dropown list disappears and dialog appears. so I am not keeping dropdown list visible. It will be hidden as current behaviour. just dialog will appear at red line mentioned by you in above comment

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Right @PrakashDurlabhji dropdown should be hidden when we click cancelled.

Now the confirmation dialog is shown under the dropdown, but it is shown on top for members. What we want here is that this confirmation dialog is inserted inside left sidebar so the content bellow it is pushed down:

image

So when confirmation dialog is opened, everything else should be still visible.

@PrakashDurlabhji
Copy link
Contributor Author

@maxceem updating

@PrakashDurlabhji
Copy link
Contributor Author

@maxceem updated

@maxceem
Copy link
Collaborator

maxceem commented Sep 24, 2019

@PrakashDurlabhji I see 2 your comments #3307 (comment) and #3307 (comment) that this PR has been updated, but there are no new commits.

Would you be able to fix this PR as per my comment above #3307 (review) or should I open this issue for pick up?

@PrakashDurlabhji
Copy link
Contributor Author

@maxceem it is completed now

@PrakashDurlabhji
Copy link
Contributor Author

@maxceem code update done and working

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Thanks @PrakashDurlabhji. The confirmation dialog now is shown as per requirements.

  1. I've noticed that Project Status is moved to the right in the dropdown after this PR:
    After PR:

    image

    Before PR:

    image

    We shouldn't move the Project Status text.

  2. Also, I'm not sure if we really need all these changes in the CSS. See comments below.

@PrakashDurlabhji
Copy link
Contributor Author

@maxceem PR update done

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

@PrakashDurlabhji there are two changes which I'm not sure if they are required or no.

  1. Change 1:

    image

  2. Change 2:

    image

@maxceem
Copy link
Collaborator

maxceem commented Sep 29, 2019

@PrakashDurlabhji there are two changes which I'm not sure if they are required or no. Could you please have a look on the comment above #3307 (review)

@maxceem
Copy link
Collaborator

maxceem commented Sep 30, 2019

@PrakashDurlabhji do you have update here?

@PrakashDurlabhji
Copy link
Contributor Author

@maxceem change 1 required to keep it center.
change 2 is not required hence skipped it

@PrakashDurlabhji
Copy link
Contributor Author

@maxceem ready

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Thanks for update @PrakashDurlabhji. Works good.

@maxceem maxceem merged commit bc2c16a into topcoder-archive:cf19 Oct 1, 2019
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.

2 participants