Skip to content

Dashboard: Issues noted by reviewers - Various problems (yet unsorted) #87

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

Closed
birdofpreyru opened this issue Jun 26, 2017 · 0 comments
Closed

Comments

@birdofpreyru
Copy link
Collaborator

birdofpreyru commented Jun 26, 2017

- There are some minor errors in console http://take.ms/SU4So

Changes broke some unit tests (as a rapid solution those test are skipped for now). Unit tests will be taken care of separately

- URLs 'bases' should not be hardcoded. You should use the configured ones.

- /src/shared/components/Dashboard/SubtrackStats/index.jsx:L(29, 33, 36, 39) you should configure the returned values and not use magic-numbers

- You should reuse the configured variables for colors in your scss instead of hardcoding the colors. Not good, but as long as it already matches the design no need to touch it right now

Not scoring for 2) on https://apps.topcoder.com/forums/?module=Thread&threadID=902686&start=0 as copilot agreed to leave it as is Split into a separate issue ticket (#106)

Copilot mentioned all links must work, one of the links points to this URL which does not load - https://cl.ly/2Y0t0z080R3B

Same as #2 - https://cl.ly/2l1P2v2x1R1v No problem, there is just no Arena in dev deployment of TC platform

Clicking on this URL does not work either - https://cl.ly/1Z2N3B3q1t2c

Spec mentions 'You should check challengeListing.pendingRequest path to see when all your requests to load challenges are completed.'
You are not using this flag

Spec mentions 'There is challengeListing.oldestData field intended to deal with it. If upon entering the Dashboard you see that this field is too far into the past, you better drop and reload all challenges loaded into Redux state. '
You are not using oldestData param at all
Fixed

Spec mentions 'If upon entering the Dashboard you see that this field is too far into the past, you better drop and reload all challenges loaded into Redux state. Sure, the actual time which triggers this should be exposed via App configuration.'
There is no such configurable field

Remaining issues were included into another dashboard challenge, and if there are still some problems, they should be logged another time in a fresher ticket with review feedbacks

- The way you display the cards (float on the right side) looks ugly http://take.ms/h4viU

UI looks odd if there are 0 challenges from the new filter - https://cl.ly/1y2517444506

Similarly, position of challenge looks odd when there is one challenge matching the filter - https://cl.ly/3Z0p0P3Z1j2h
It's too close to the filter and there are empty space on the other side

Why are the communities empty - https://cl.ly/1c1247032n06

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

No branches or pull requests

1 participant