Skip to content
This repository has been archived by the owner on Apr 2, 2021. It is now read-only.

UI Touchup #23

Merged
merged 27 commits into from
May 3, 2020
Merged

UI Touchup #23

merged 27 commits into from
May 3, 2020

Conversation

codetheweb
Copy link
Collaborator

@codetheweb codetheweb commented Apr 24, 2020

Highlights:

  • Added linter (can swap it out if you want, I like it because it's strict)
  • Converted class-based components to functional components
  • Dark mode (changes based on system setting)
  • A few UI tweaks
  • Removed Thunk dependency for Redux, simplifies code

Preview:
Screen Shot 2020-04-23 at 8 19 46 PM

Some ideas for further development:

  • Mobile is garbage. Gotta figure out a good way to display tables. After increasing responsiveness, it's not actually that bad. The table works fine with horizontal scrolling.
  • Increase performance when searching. @Blu3spirits suggested using a debouncer on user input. I changed the default table length to 10 rows and that seems to help as well, although obviously not an ideal solution.
  • Make API endpoint URL an env var so we can set it at build time.
  • In line with the above, make some kind of script that creates a static build and links it with Django. Edit: maybe? I guess they recommend using a different server for static files.
  • In the screenshot, you may see I added a 'last updated at...' value. I think this would be helpful for people who use the site but may think it was abandoned and doesn't have up-to-date information.

@Blu3spirits
Copy link
Collaborator

Going to have to put this on the backburner for now as exams are this week.
Planning to get back to this Wed. Apr 29

@codetheweb
Copy link
Collaborator Author

Used a combination of debouncing and memoization to significantly improve responsiveness. Feels much snappier, especially on mobile.

@codetheweb codetheweb force-pushed the feature/ui-touchup branch 2 times, most recently from c8f766a to 91ecb73 Compare April 25, 2020 18:30
@Blu3spirits Blu3spirits added the enhancement New feature or request label Apr 29, 2020
@Blu3spirits
Copy link
Collaborator

In line with the above, make some kind of script that creates a static build and links it with Django. Edit: maybe? I guess they recommend using a different server for static files.

typically it is recommended to do this. However, using a module such as whitenoise is supported and "okay" to do. I'm going to handle doing this one as it's on the Django side and would help solve a deployment hang-up I've been having.

Copy link
Collaborator

@Blu3spirits Blu3spirits left a comment

Choose a reason for hiding this comment

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

I really like the dark theme idea you've put forward. Definitely a lot easier on the eyes and with debounced input the UX is vastly improved.

webapp/src/ui/components/courses-list.js Show resolved Hide resolved
webapp/src/index.js Outdated Show resolved Hide resolved
webapp/src/store/actions/courses.js Outdated Show resolved Hide resolved
webapp/src/ui/courses.js Outdated Show resolved Hide resolved
webapp/src/ui/page.js Outdated Show resolved Hide resolved
webapp/src/ui/courses.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@Blu3spirits Blu3spirits left a comment

Choose a reason for hiding this comment

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

Good work!

I'm going to attempt to deploy this real quick before I give my approval

@codetheweb codetheweb requested a review from Kiro47 May 2, 2020 20:18
@Kiro47
Copy link
Owner

Kiro47 commented May 3, 2020

I did find one minor bug, but it can be moved to another issue if you feel like dealing with it later.

Recreation:

  1. Do normal development setup
  2. Go to web page
  3. Go up a few pages to where you're in a higher results list. (In example, I went to "Rows per Page=200" then went to the 4th page "Entries 601-800".
  4. Search for something with less results than the lowest entry. (An example being "Dynamics" with 72 results).
  5. On load it'll still have you on the page with 601-800 entries, and to get to your entries you'll just have to keep going back pages unless you're on actual values.

Proposed Solution:
On search query reset the page to page 1 with entries "1 to Max_Rows".

@codetheweb
Copy link
Collaborator Author

I can fix that tonight.

Looks like I also have to start signing commits.

@codetheweb
Copy link
Collaborator Author

codetheweb commented May 3, 2020

Fixed @Kiro47. Edit: still working on signing old commits.

@Blu3spirits
Copy link
Collaborator

If you wanted you could probably just rebase if you want. It's not exactly "best practice" but it will get everything under one signed commit

@codetheweb codetheweb force-pushed the feature/ui-touchup branch 3 times, most recently from 8f4eb1a to a599741 Compare May 3, 2020 01:57
@Kiro47
Copy link
Owner

Kiro47 commented May 3, 2020

Looks like you missed the topmost commit when resigning. To make it to resign here's a cheap little snippet:

git filter-branch -f --commit-filter 'git commit-tree -S "$@";' 6ac2b59359de934bc10252a76e4c39b2d9a57377^1..HEAD

It'll resign all the commits from that hash (6ac2b59359de934bc10252a76e4c39b2d9a57377) and on. Since each commit hash is based on the old one, you unfortunately have to resign all the commits after modifying even one, this makes it a bit easier.
Also, make sure you keep that ^1, otherwise it will gpg resign everything after that commit but skip that commit itself.

@codetheweb codetheweb force-pushed the feature/ui-touchup branch 2 times, most recently from a6c8e0a to 8392382 Compare May 3, 2020 16:27
@codetheweb codetheweb force-pushed the feature/ui-touchup branch from 929588a to b9cc804 Compare May 3, 2020 16:38
@codetheweb
Copy link
Collaborator Author

Sorry about that @Kiro47.

Looks like it's good to go now.

@Blu3spirits Blu3spirits merged commit 4e13ee6 into Kiro47:master May 3, 2020
@codetheweb codetheweb deleted the feature/ui-touchup branch May 3, 2020 17:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants