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

Basic responsive layout + minor style upgrades #1

Merged
merged 11 commits into from
Mar 25, 2022

Conversation

jhuddle
Copy link
Contributor

@jhuddle jhuddle commented Mar 16, 2022

Hi Andrew - great project, thanks for doing this!

I saw your call-out on FB for collaboration on the UI side of things, so this PR adds some basic responsivity (mostly for the benefit of mobile users) plus some small style upgrades. I've kept it reasonably faithful to your original style so far, and I've not touched the original main.css or element layout at all yet - but I think the HTML could benefit from some semantic restructuring for accessibility purposes and to unlock more creative possibilities with the CSS. Happy to work on this further with you if you're up for it! :)

  • Jamie

@qwandor
Copy link
Owner

qwandor commented Mar 22, 2022

Thanks a lot Jamie! I've been busy the last few days with some other things, but this looks pretty good. I'll try to have a proper look soon and get it merged, I just want to tweak a few things first.

@qwandor
Copy link
Owner

qwandor commented Mar 25, 2022

I think I'm happy enough with this now to merge it and push it to production, but there are a few issues I'd still like to fix:

  • For some reason, putting the table row borders on the tr rather than the td results in them being an inconsistent width.
  • The table layout goes weird at some narrower screen widths. I guess this is to do with the grid layout thing you do, I don't know how that works.
  • At some intermediate screen widths it breaks some longer dance style names (e.g. "English ceilidh") across two lines, and makes them wider than they should be.

@qwandor qwandor merged commit 1115f4d into qwandor:main Mar 25, 2022
@jhuddle
Copy link
Contributor Author

jhuddle commented Mar 30, 2022

Great - I see you moved a few things around after the merge too, did that fix everything or is there still anything you'd like me to look at for a future PR?

@qwandor
Copy link
Owner

qwandor commented Mar 30, 2022

I think I've got the grid layout looking better now, but the other two issues are still there. If you have any thoughts they would be welcome.

@qwandor
Copy link
Owner

qwandor commented Apr 26, 2022

I've filed #3 and #4 to track the remaining issues from this.

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