-
Notifications
You must be signed in to change notification settings - Fork 14
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
feature flag details page overview header left code changes #1601
feature flag details page overview header left code changes #1601
Conversation
@@ -20,6 +20,27 @@ export class FeatureFlagsService { | |||
isAllFlagsFetched$ = this.store$.pipe(select(selectIsAllFlagsFetched)); | |||
activeDetailsTabIndex$ = this.store$.pipe(select(selectActiveDetailsTabIndex)); | |||
|
|||
formatDateString(dateString, dateTime) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have dayjs
for date formatting:
const date = dayjs("2024-05-31T21:54:19.692Z"); date.format('MMM DD, YYYY, hh:mm A');
But we also already have a FormatDate
pipe. We should probably just use that to be consistent with how this and other dates are done. I wouldn't worry too much about getting it exactly like in the design, but if you want to create an additional format, we should use the pipe to do that as well.
<span class="ft-14-700 flag-create-date">
{{ ('home.view-experiment.created-on.text' | translate) + (flag.createdAt | formatDate: 'medium date') }}
</span>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed this cmt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, this function can be deleted then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment on the date format function is only thing, otherwise looks good
I made the change but it's not the same as shown in the design as it has only 2 options, we can add more options in the pipe later to provide more variety as per the needs. |
This PR contains changes related to the details page overview header left integration.