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

[frontend] fixed calendar button alignment in Oozie WF Submit form #3740

Merged
merged 6 commits into from
Oct 24, 2024

Conversation

tarunjangid
Copy link
Contributor

@tarunjangid tarunjangid commented May 17, 2024

What changes were proposed in this pull request?

  • Ref: CDPD-66097
  • This PR aims to resolve 2 issues in the Submit form:
  1. When the screen is zoom out, the calendar button does not align if the number of arguments is more than 3. If we zoom in the button should get aligned to its original place, which does not happen at the moment.
  2. The calendar does not hide if we click anywhere outside calendar control.

How was this patch tested?

  • manually tested.
  • attaching screen recording of before and after.

Before:

before_changes.mov

After:

after_changes.mov

Please review Hue Contributing Guide before opening a pull request.

@Harshg999
Copy link
Collaborator

Thanks @tarunjangid for contributing a fix! I've tagged the UI code owners for reviewing this PR.
On a side note, please don't add the link to internal JIRAs in PR description.

position: fixed;
}
#param-container .btn-group.open {
position: absolute !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need !important here? If so perhaps some other class is more specific...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed this #param-container .btn-group.open style.
Earlier to acknowledge the issue where the calendar button moved from its place when we zoomed in, I added the position as fixed. Because of this fixed position, when we opened the calendar button it overlapped with the button below that (as shown in the attached image), hence I used the !important with position style of #param-container .btn-group.open.

The ideal solution would be to have an absolute position for the #param-container .btn-group style.

Screenshot 2024-06-13 at 10 18 16 PM

@@ -168,6 +172,14 @@ else:
huePubSub.subscribeOnce('hide.datepicker', function () {
_el.datepicker('hide');
});
$(document).on("click.hideDatepicker", function (event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This attaches an event listener on every click ($(".calendera-link").on("click" ... above). Better to attach it on show and detach on hide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made code changes

_el.datepicker('hide');
}
});
$(".calendar-link, input[type='text']").on("click.stopPropagation", function(event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made code changes

Comment on lines 168 to 170
$(document).off("click.hideDatepicker", function (event) {
event.stopPropagation();
});
Copy link
Contributor

Choose a reason for hiding this comment

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

This has no effect. When a function is provided to off it must be equal to the function provided in the on call. To remove all event handlers do $(document).off("click.hideDatepicker");

@tarunjangid tarunjangid marked this pull request as draft July 22, 2024 04:09
Copy link

github-actions bot commented Sep 6, 2024

This PR is stale because it has been open 45 days with no activity and is not labeled "Prevent stale". Remove "stale" label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Sep 6, 2024
@Harshg999 Harshg999 removed the Stale label Sep 9, 2024
_el.datepicker('hide');
}
});
$(document).off("click.hideDatepicker");
Copy link
Contributor

Choose a reason for hiding this comment

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

This will remove the event handler right after it's been added on line 170. It's probably not what you intend.

Copy link
Contributor

@JohanAhlen JohanAhlen left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks! :-)

@JohanAhlen JohanAhlen merged commit 9a85afa into cloudera:master Oct 24, 2024
6 checks passed
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.

4 participants