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

Added support to configure date time format from settings page and updated UI of settings page(#2t2y3qq) #60

Merged
merged 28 commits into from
Nov 15, 2022

Conversation

disha1202
Copy link
Contributor

@disha1202 disha1202 commented Sep 14, 2022

Closes #87

Screenshot from 2022-09-19 13-50-21

Screenshot from 2022-09-19 13-38-23

Screenshot from 2022-09-27 12-04-20

@disha1202 disha1202 marked this pull request as draft September 14, 2022 11:51
@disha1202 disha1202 marked this pull request as ready for review September 19, 2022 06:14
@disha1202 disha1202 changed the title [WIP]Added support to configure date time format from settings page(#2t2y3qq) Added support to configure date time format from settings page(#2t2y3qq) Sep 19, 2022
@disha1202 disha1202 requested a review from dt2patel September 19, 2022 06:14
src/components/DateTimeModal.vue Outdated Show resolved Hide resolved
src/components/DateTimeModal.vue Outdated Show resolved Hide resolved
Copy link
Contributor

@dt2patel dt2patel left a comment

Choose a reason for hiding this comment

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

See comments

src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
@disha1202 disha1202 changed the title Added support to configure date time format from settings page(#2t2y3qq) Added support to configure date time format from settings page and updated UI of settings page(#2t2y3qq) Sep 19, 2022
src/views/Settings.vue Outdated Show resolved Hide resolved
src/store/modules/user/getters.ts Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/store/modules/user/getters.ts Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/views/Settings.vue Outdated Show resolved Hide resolved
src/store/modules/user/actions.ts Outdated Show resolved Hide resolved
src/views/OrderDetail.vue Outdated Show resolved Hide resolved
Copy link
Contributor

@ymaheshwari1 ymaheshwari1 left a comment

Choose a reason for hiding this comment

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

Add comments when formatting the date time using luxon

@@ -3,4 +3,5 @@ export default interface UserState {
current: object | null;
currentFacility: object;
instanceUrl: string;
dateTimeFormat: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dateTimeFormat: string;
preferredDateTimeFormat: string;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated variable name.

@@ -18,6 +18,7 @@ const actions: ActionTree<UserState, RootState> = {
const resp = await UserService.login(username, password)
if (resp.status === 200 && resp.data) {
if (resp.data.token) {
dispatch('setDateTimeFormat', process.env.VUE_APP_DATE_FORMAT ? process.env.VUE_APP_DATE_FORMAT : 'MM/dd/yyyy');
Copy link
Contributor

Choose a reason for hiding this comment

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

This is should be done after successful login only, after permission check

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 the requested change.

@@ -212,6 +213,10 @@ export default defineComponent({
this.fetchFacilities();
},
methods: {
isDateInvalid(){
// Checked if any of the date format is different than the selected format.
return !this.ordersList.items.some((item: any) => DateTime.fromFormat(item.arrivalDate, this.dateTimeFormat).isValid);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the condition should be inverted while checking items, if any one of the item is invalid it should return

Suggested change
return !this.ordersList.items.some((item: any) => DateTime.fromFormat(item.arrivalDate, this.dateTimeFormat).isValid);
return this.ordersList.items.some((item: any) => !DateTime.fromFormat(item.arrivalDate, this.dateTimeFormat).isValid);

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 the requested change.

@adityasharma7 adityasharma7 merged commit fa6c51c into hotwax:main Nov 15, 2022
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.

Update the UI of settings page and add support to configure date time format
4 participants