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: Part three bug bash custom messages #25229

Merged
merged 14 commits into from
Feb 7, 2024

Conversation

kiannaquach
Copy link
Contributor

@kiannaquach kiannaquach commented Feb 5, 2024

  • Update the message type to be gray.
Screenshot 2024-02-05 at 3 22 07 PM
  • Add namespace reminder and remove form description.
Screenshot 2024-02-05 at 3 22 20 PM
  • Update the language to include the full word, information.
Screenshot 2024-02-05 at 3 22 27 PM
  • Removes start and end time from the serializer hash!
Screenshot 2024-02-06 at 9 23 21 AM
  • Rest pageFilter when exiting

  • Add start and end time client side validation. Fixes issue where start time was setting date to 1969 when user types in date / time with a prefix of 0.
    custom-messages-mulitple

@kiannaquach kiannaquach added this to the 1.16.0-rc1 milestone Feb 5, 2024
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Feb 5, 2024
@kiannaquach kiannaquach marked this pull request as ready for review February 6, 2024 21:54
Copy link

github-actions bot commented Feb 6, 2024

CI Results:
All Go tests succeeded! ✅

Copy link

github-actions bot commented Feb 6, 2024

Build Results:
All builds succeeded! ✅

Comment on lines +29 to +31
const start = new Date(model.startTime);
const end = new Date(model.endTime);
return isBefore(start, end);
Copy link
Contributor

Choose a reason for hiding this comment

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

If the start and end times are ISO strings you can pass those directly to isBefore without needing to create date objects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These strings are in datetime local format. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. isBefore should accept that format without needing to convert to a date object but just more of an FYI than anything 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good to know I'll update that!

Comment on lines +49 to +50
this.messageEndTime = e.target.value;
this.args.message.endTime = this.messageEndTime;
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why you are tracking the endTime with a local variable rather than directly mutating the property on args.message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll need some way to remember the old endTime that was selected. If a user selects a specific date, and decides to click 'never', but then goes back to select specific date they'll have their previous state.

@kiannaquach kiannaquach merged commit 4283caa into main Feb 7, 2024
31 checks passed
@kiannaquach kiannaquach deleted the ui/VAULT-23799/part-three-custom-message-bugs branch February 7, 2024 18:43
Monkeychip pushed a commit that referenced this pull request Feb 12, 2024
* Address comments

* Fix serailizer warning mesage

* Reset pageFilter when exiting

* Add start and end time validation and fix bugs

* Fix failing tests

* Add validation tests

* Set end time in contorller

* Address feedback

* Remove new date

* Put new Date back
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-changelog ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants