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

chore: upgrade to Fresh 1.6.1 and std 0.208.0 #646

Merged
merged 7 commits into from
Dec 13, 2023
Merged

Conversation

deer
Copy link
Contributor

@deer deer commented Dec 6, 2023

closes #644

I see the guidelines have changed since I last contributed (thanks github!) and that caused me to look for an issue. Fortunately there was the above, but I see you want tailwind as well. I'll open this as a draft for now.

@iuioiua iuioiua changed the title chore: uptake fresh 1.6.0 and std 0.208.0 chore: upgrade to Fresh 1.6.0 and std 0.208.0 Dec 6, 2023
@deer deer marked this pull request as ready for review December 6, 2023 09:48
@deer
Copy link
Contributor Author

deer commented Dec 6, 2023

@iuioiua, this is ready to go now. Although you might consider some style tests somewhere. I almost botched this by forgetting to add the styles.css file. Of course the site was unstyled, but all the tests were passing.

@iuioiua
Copy link
Contributor

iuioiua commented Dec 6, 2023

Pretty much LGTM! I'll give this a spin tomorrow before approving.

As for the styles test, let's create an issue to fix after we land this PR.

@iuioiua iuioiua changed the title chore: upgrade to Fresh 1.6.0 and std 0.208.0 chore: upgrade to Fresh 1.6.1 and std 0.208.0 Dec 7, 2023
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. I've updated to Fresh 1.6.1. But I've noticed a couple of visual issues:

  1. The top menu bar is no longer visible
  2. The social icons in the bottom-right corner are squashed together.

@marvinhagemeister marvinhagemeister dismissed their stale review December 7, 2023 21:47

haven't checked for the issues iuioiua mentioned. Thus dismissing my review.

@deer
Copy link
Contributor Author

deer commented Dec 7, 2023

Thanks guys. I'll try again on Friday, first ensuring a near pixel perfect match. I guess I need to familiarize myself with the app's functionality 😅

@deer
Copy link
Contributor Author

deer commented Dec 8, 2023

Some progress here, but it's a slog. I had to hack it by leveraging the safelist in the config, so obviously something is majorly wrong. Additionally I haven't managed to fix the inputs on the submit page. (They're appearing as some sort of medium gray when not logged in, but I think they should be the same as the background color.)

On the positive side, I'm obviously deep in the code checking out all the pages. So maybe I'll finally have something else to say in this repo after (if!?) I fix this 🙃

If either of you have any great ideas, I'm ready to be saved. Otherwise I'll continue later.

@iuioiua
Copy link
Contributor

iuioiua commented Dec 8, 2023

Hm... I think Tailwind can't recognise classes if stored in variables. The fix may be as simple as removing these "class variables", like LINK_STYLES, and just using the classes directly. We can also create custom classes and reuse those.

@deer deer marked this pull request as draft December 10, 2023 23:45
@deer
Copy link
Contributor Author

deer commented Dec 10, 2023

Still fighting with active links, but it's overall much better now.

@deer deer marked this pull request as ready for review December 12, 2023 12:50
@deer
Copy link
Contributor Author

deer commented Dec 12, 2023

  • As per https://tailwindcss.com/docs/border-width, tailwind does not do border-1, so I had to define this in tailwind.config.ts.
  • The solution to the link issue was to change the position of the exclamation mark.
  • Somehow tw-ring-opacity: 0.5; was getting inserted into the old version automatically; I had to manually apply it here with ring-opacity-50.

Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

LGTM! Awesome work, @deer! Thanks for this.

@iuioiua iuioiua merged commit 27320c5 into denoland:main Dec 13, 2023
6 checks passed
@deer deer deleted the uptake_1.6 branch December 15, 2023 12:59
iuioiua pushed a commit that referenced this pull request Feb 23, 2024
"Someone" broke the hamburger menu in #646.

This fixes the issue and cleans things up by making the CSS a bit more
direct:
* I use
https://tailwindcss.com/docs/hover-focus-and-other-states#styling-based-on-sibling-state
to modify the `nav` directly.
* I add IDs to the hamburger and X components, and then use
https://tailwindcss.com/docs/hover-focus-and-other-states#using-arbitrary-variants
to directly target them, in combination with peer.

FYI @hashrock since this restores the great work you did in #374.
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.

todo: upgrade to Fresh 1.6
3 participants