Skip to content

Replace loading pie with loadingbar#273

Open
julian-ao wants to merge 3 commits intomainfrom
loading-bar
Open

Replace loading pie with loadingbar#273
julian-ao wants to merge 3 commits intomainfrom
loading-bar

Conversation

@julian-ao
Copy link
Copy Markdown
Member

@julian-ao julian-ao commented Mar 29, 2025

Removed the loading pie in the header, and replaced it with a thin loading bar on either the top ro bottom on the screen.

Need input. Place it on top or bottom?

image image

@julian-ao julian-ao self-assigned this Mar 29, 2025
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 29, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
infoskjerm-backend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 9, 2025 0:30am
infoskjerm-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 9, 2025 0:30am

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the circular progress pie with a horizontal loading bar for visual progress indication. Key changes include:

  • Integrating a new LoadingWrapper component in MainPage.
  • Removing the CircularProgressbar and associated properties from Header.
  • Adding the LoadingWrapper component with custom striped styling and an onClick navigation trigger.

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

File Description
frontend/src/components/pages/MainPage.tsx Imports LoadingWrapper and adds it alongside Header for loading.
frontend/src/components/header/Header.tsx Removes CircularProgressbar and related props, simplifying Header.
frontend/src/components/LoadingWrapper.tsx Introduces a new loading bar component with time-based progress.
Files not reviewed (1)
  • frontend/package.json: Language not supported

Comment on lines +15 to +21
const [startTime, setStartTime] = useState<number | null>(null)

useEffect(() => {
setStartTime(Date.now())

const interval = setInterval(() => {
const elapsed = Date.now() - (startTime || Date.now())
Copy link

Copilot AI Mar 29, 2025

Choose a reason for hiding this comment

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

The useEffect callback may capture a stale value of startTime from the closure, causing inaccurate progress calculations. Consider initializing the start time using a ref or setting it outside the interval callback to ensure consistent timing.

Suggested change
const [startTime, setStartTime] = useState<number | null>(null)
useEffect(() => {
setStartTime(Date.now())
const interval = setInterval(() => {
const elapsed = Date.now() - (startTime || Date.now())
const startTime = useRef<number | null>(null)
useEffect(() => {
startTime.current = Date.now()
const interval = setInterval(() => {
const elapsed = Date.now() - (startTime.current || Date.now())

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

what did youn smoke

@SylviaSWYung
Copy link
Copy Markdown
Contributor

Mhmmmm, jeg syns pien var søt... og baren var litt harry. Hvorfor bytter vi fra pie til bar?

@julian-ao
Copy link
Copy Markdown
Member Author

Synes ikke paien var så fin. Og baren kan vises på sider som er fullscreen uten headeren.

@julian-ao
Copy link
Copy Markdown
Member Author

Hvis det hjelper kan vi jo endre opacity'en på den
image

@SylviaSWYung
Copy link
Copy Markdown
Contributor

Hvis det hjelper kan vi jo endre opacity'en på den
image

Skjønner med pien. Syns bare det r litt harry med to farger, minner meg om "utlandet barber shop candy cane". Men kanskje vi kan ta en bestemt farge i en annen opacity? Som gjør at den skiller seg litt ut men samtidig ikke?

@julian-ao
Copy link
Copy Markdown
Member Author

Hva tenker du @jorgengaldal ?

@jorgengaldal
Copy link
Copy Markdown
Contributor

I utgangspunktet litt enig med Sylvia i at det ser litt halvrart ut. Kunne det vært en idé at man heller har progressbar langs hele kanten både oppe og nede fra topp venstre til bunn høyre, sånn som det er på noen sponsede segmenter på youtube?

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.

4 participants