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

Feedback/#11253 #11304

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Feedback/#11253 #11304

wants to merge 16 commits into from

Conversation

piggydoughnut
Copy link
Contributor

@piggydoughnut piggydoughnut commented Feb 17, 2025

Addressing feedback from #11253 and a few points from #11262 (comment)

changes

  • fixed issue with start and end dates on workload and workplans being the same (Coretime page is very confusing #11253 (comment)) (Coretime Chain -> Coretime )
  • on Coretime Chain -> filter by parachain id in both workload and workplan, so users can see where is their chain scheduled in the future
  • changed cycle progress componenet to show countdown -> now you can see both countdown in time and timeslice progress with a pie chart (Relay chain -> Coretime -> Overview)
  • Sale tab - showing start price of the sale in the Overview
  • Sale tab - always showing price even if the cores are sold out
  • Sale tab - sale history - fix for the dates for the Fixed price period (currently fixed price period has same start and end date)
    to reproduce:
    • Go to Sale tab under Coretime on Relay chain
    • Choose sale number in the select at the bottom
    • Look at the last table on the left which is called Fixed price Phase (start and end should be 1 week apart)

minor code changes

  • renamed chainName to relayName

@piggydoughnut piggydoughnut marked this pull request as draft February 17, 2025 07:58
@piggydoughnut piggydoughnut marked this pull request as ready for review March 5, 2025 06:38
Copy link
Contributor

@ap211unitech ap211unitech left a comment

Choose a reason for hiding this comment

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

Overall, it looks great to me 🚀
Just have a couple of questions and suggestions.

@@ -267,7 +267,7 @@ export const getSaleParameters = (
coretime: 0,
relay: currentRegionInfo.end.blocks.relay + get.blocks.relay(config.regionLength)
},
date: estimateTime(currentRegionInfo.end.ts + config.regionLength, get.blocks.relay(lastCommittedTimeslice), constants.relay),
date: formatDate(new Date(estimateTime(currentRegionInfo.end.ts + config.regionLength, get.blocks.relay(lastCommittedTimeslice), constants.relay) ?? '')),
Copy link
Contributor

Choose a reason for hiding this comment

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

It might end up giving Invalid Date or some incorrect info if

const estimated = estimateTime(currentRegionInfo.end.ts + config.regionLength, get.blocks.relay(lastCommittedTimeslice), constants.relay);

if estimated returns null and the expression will eventually evaluate to formatDate(new Date('')).

However, If it is always guaranteed that estimatedTime will not return null, then we should update types accordingly.

@@ -164,11 +164,11 @@ const getPhaseConfiguration = (
const fixedPriceLengthTs = regionLength - interludeLengthTs - leadInLengthTs;
const fixedPriceEndTs = priceDiscoveryEndTs + fixedPriceLengthTs;
const get = createGet(constants);
const getDate = (ts: number) => estimateTime(ts, get.blocks.relay(lastCommittedTimeslice) ?? 0, constants.relay);
const getDate = (ts: number) => formatDate(new Date(estimateTime(ts, get.blocks.relay(lastCommittedTimeslice) ?? 0, constants.relay) ?? ''));
Copy link
Contributor

@ap211unitech ap211unitech Mar 6, 2025

Choose a reason for hiding this comment

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

const chainRegionEnd = (chainRecord.renewalStatus === ChainRenewalStatus.Renewed ? regionEnd : regionBegin);
const targetTimeslice = lease?.until || chainRegionEnd;
const showEstimates = !!targetTimeslice && Object.values(CoreTimeTypes)[chainRecord.type] !== CoreTimeTypes.Reservation;
const { coretimeInfo, get } = useCoretimeContext();

const estimatedTime = showEstimates && get && coretimeInfo &&
estimateTime(targetTimeslice, get.blocks.relay(lastCommittedTimeslice), coretimeInfo?.constants?.relay);
formatDate(new Date(estimateTime(targetTimeslice, get.blocks.relay(lastCommittedTimeslice), coretimeInfo?.constants?.relay) ?? ''));
Copy link
Contributor

@ap211unitech ap211unitech Mar 6, 2025

Choose a reason for hiding this comment

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

const currentRegionStart = useMemo(() => coretimeInfo ? coretimeInfo.salesInfo.regionEnd - coretimeInfo?.config.regionLength * 2 : 0, [coretimeInfo]);

const saleStartDate = useMemo(() => {
return get && coretimeInfo && formatDate(new Date(estimateTime(currentRegionStart, get.blocks.relay(coretimeInfo?.status?.lastTimeslice), coretimeInfo.constants.relay) ?? ''), true);
Copy link
Contributor

@ap211unitech ap211unitech Mar 6, 2025

Choose a reason for hiding this comment

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

}, [currentRegionStart, coretimeInfo, get]);

const saleEndDate = useMemo(() => {
return get && coretimeInfo && formatDate(new Date(estimateTime(currentRegionEnd, get.blocks.relay(coretimeInfo?.status?.lastTimeslice), coretimeInfo.constants.relay) ?? ''), true);
Copy link
Contributor

@ap211unitech ap211unitech Mar 6, 2025

Choose a reason for hiding this comment

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

@@ -46,154 +46,62 @@ const TableCol = ({ header,
);

function WorkInfoRow ({ data }: { data: InfoRow }): React.ReactElement {
if (!data.task) {
if (!data.task || data.type === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can say it just !data.type.

Copy link
Contributor Author

@piggydoughnut piggydoughnut Mar 10, 2025

Choose a reason for hiding this comment

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

data.type is an enum, hence it starts from 0 :) Actually I just realized this check is due to a switch later down the file, that is removed now, so I will remove this too.

export enum CoreTimeTypes {
  'Reservation',
  'Lease',
  'Bulk Coretime',
  'On Demand'
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright. Thanks.

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.

2 participants