Copilot commented on code in PR #494:
URL:
https://github.com/apache/skywalking-booster-ui/pull/494#discussion_r2300507964
##########
src/components/TimePicker.vue:
##########
@@ -110,15 +140,29 @@ limitations under the License. -->
</template>
<script lang="ts" setup>
- import { ref, computed, onMounted, onBeforeUnmount, watch } from "vue";
+ import { ref, computed, onMounted, onBeforeUnmount, watch, PropType } from
"vue";
import { useI18n } from "vue-i18n";
import DateCalendar from "./DateCalendar.vue";
import { useTimeoutFn } from "@/hooks/useTimeout";
- /*global PropType, defineProps, defineEmits*/
+ /*global, defineProps, defineEmits*/
+
+ const QUICK_PICK_TYPES = {
+ QUARTER: "quarter",
+ HALF: "half",
+ HOUR: "hour",
+ DAY: "day",
+ WEEK: "week",
+ MONTH: "month",
+ } as const;
+
+ type QuickPickType = typeof QUICK_PICK_TYPES[keyof typeof QUICK_PICK_TYPES];
+
const datepicker = ref(null);
const { t } = useI18n();
const show = ref<boolean>(false);
const dates = ref<Date[]>([]);
+ const inputDates = ref<Date[]>([]);
+ const selectedShortcut = ref<string>(QUICK_PICK_TYPES.HALF);
Review Comment:
The selectedShortcut is hardcoded to initialize with 'HALF'. Consider
initializing it based on the current props.value to reflect the actual selected
time range, or make it nullable to indicate no initial selection.
```suggestion
// Helper function to determine shortcut type from value
function getShortcutFromValue(value: any): string | null {
// Example logic: match value to shortcut type
// This should be customized based on how value maps to shortcuts
if (!value) return null;
// If value is a string and matches a shortcut type
if (typeof value === "string") {
const lower = value.toLowerCase();
for (const key in QUICK_PICK_TYPES) {
if (QUICK_PICK_TYPES[key].toLowerCase() === lower) {
return QUICK_PICK_TYPES[key];
}
}
}
// If value is a Date or Array, add logic as needed
// For now, default to null
return null;
}
const selectedShortcut = ref<string |
null>(getShortcutFromValue(props.value));
```
##########
src/components/TimePicker.vue:
##########
@@ -246,44 +289,47 @@ limitations under the License. -->
const dc = (e: MouseEvent) => {
show.value = (datepicker.value as any).contains(e.target) &&
!props.disabled;
};
- const quickPick = (type: string) => {
- const end = new Date();
- const start = new Date();
+ const quickPick = (type: QuickPickType) => {
+ const end = new Date(props.maxRange[1]);
+ const start = new Date(props.maxRange[1]);
Review Comment:
Using props.maxRange[1] as the end time for quick picks may not be the
intended behavior. The original code used `new Date()` (current time) as the
end time, which is more typical for 'last N minutes/hours' functionality. This
change makes all quick picks end at the maximum allowed time rather than the
current time.
```suggestion
const end = new Date();
const start = new Date();
```
##########
src/components/TimePicker.vue:
##########
@@ -110,15 +140,29 @@ limitations under the License. -->
</template>
<script lang="ts" setup>
- import { ref, computed, onMounted, onBeforeUnmount, watch } from "vue";
+ import { ref, computed, onMounted, onBeforeUnmount, watch, PropType } from
"vue";
import { useI18n } from "vue-i18n";
import DateCalendar from "./DateCalendar.vue";
import { useTimeoutFn } from "@/hooks/useTimeout";
- /*global PropType, defineProps, defineEmits*/
+ /*global, defineProps, defineEmits*/
Review Comment:
The comment syntax is malformed. It should be `/*global defineProps,
defineEmits*/` - the comma after 'global' should be removed.
```suggestion
/* global defineProps, defineEmits */
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
