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

feat(readwise): init #1130

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Conversation

goooseman
Copy link

@goooseman goooseman commented Jul 28, 2024

🎉 Theme for Readwise 🎉

Readwise helps you get the most out of what you read by making it fun & easy to revisit your highlights from all your favorite reading platforms in one place. Quickly synchronize your highlights from Kindle, Apple Books, Instapaper, Pocket, Medium, Goodreads, and even paper books.

preview

🗒 Checklist 🗒

  • I have read and followed Catppuccin's submission guidelines.
  • I have made a new directory underneath /styles/<name-of-website> containing the contents of the /template directory.
    • I have ensured that the new directory is in lower-kebab-case.
    • I have followed the template and kept the preprocessor as LESS.
  • I have made sure to update the
    userstyles.yml
    file with information about the new userstyle.
  • I have included the following files:
    • catppuccin.user.css - all the CSS for the userstyle, based on the
      template.
    • preview.webp - composite image of all four individual flavor screenshots stitched together,
      generated via Catwalk.

@goooseman goooseman requested a review from a team as a code owner July 28, 2024 20:05
@goooseman goooseman changed the title feat(readwise): creates user style feat(readwise): init Jul 28, 2024
@goooseman
Copy link
Author

Closes catppuccin/catppuccin#2479

scripts/userstyles.yml Outdated Show resolved Hide resolved
styles/readwise/catppuccin.user.css Show resolved Hide resolved
styles/readwise/catppuccin.user.css Outdated Show resolved Hide resolved
styles/readwise/catppuccin.user.css Outdated Show resolved Hide resolved
styles/readwise/catppuccin.user.css Outdated Show resolved Hide resolved
styles/readwise/catppuccin.user.css Outdated Show resolved Hide resolved
@goooseman
Copy link
Author

@uncenter Thank you for the review, I've approved all the changed, the only doubt is about the domain change, as I've described in the thread there.

@uncenter
Copy link
Member

Please don't update this branch unnecessarily!

Copy link
Member

@uncenter uncenter left a comment

Choose a reason for hiding this comment

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

Going off of the preview image you have provided, a few things need work. The grid of blue and white squares should be themed. The text color should be text instead of rosewater. The lightning bolt in the top right should be themed.

scripts/userstyles.yml Outdated Show resolved Hide resolved
styles/readwise/catppuccin.user.css Outdated Show resolved Hide resolved
styles/readwise/catppuccin.user.css Outdated Show resolved Hide resolved
@goooseman
Copy link
Author

Before generating new screenshot I want to verify the colors are ok (the color of the lightning icon is the last thing I did not complete yet)

Latte

Screenshot 2024-08-02 at 15 11 36 Screenshot 2024-08-02 at 15 11 43

Macchiato

Screenshot 2024-08-02 at 15 12 06 Screenshot 2024-08-02 at 15 12 22

Mocha

Screenshot 2024-08-02 at 15 12 36 Screenshot 2024-08-02 at 15 12 43

Latte

Screenshot 2024-08-02 at 15 13 39 Screenshot 2024-08-02 at 15 13 43

@goooseman
Copy link
Author

@uncenter @isabelroses all the comments are resolved, preview image is updated both in the PR content and description

styles/readwise/catppuccin.user.css Show resolved Hide resolved
styles/readwise/preview.webp Outdated Show resolved Hide resolved
styles/readwise/catppuccin.user.css Outdated Show resolved Hide resolved
@goooseman
Copy link
Author

@uncenter @isabelroses PR is updated, comments are resolved

@uncenter
Copy link
Member

No need to force push FYI. We squash merge so your commits in PRs don't matter much.

isabelroses
isabelroses previously approved these changes Aug 10, 2024
Copy link
Member

@uncenter uncenter left a comment

Choose a reason for hiding this comment

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

I'll continue editing the comment below for the next few minutes as I explore the application.

CleanShot 2024-08-10 at 17 55 31@2x
Seeing some unthemed things on the first page after creating an account, this import highlights one.

CleanShot 2024-08-10 at 17 57 50@2x

styles/readwise/catppuccin.user.css Outdated Show resolved Hide resolved
@goooseman
Copy link
Author

@uncenter thank you for your help and careful review, I will resolve those design issues soon and will update my PR

@goooseman
Copy link
Author

@uncenter changes are applied
Screenshot 2024-08-19 at 11 15 31
Screenshot 2024-08-19 at 11 15 28
Screenshot 2024-08-19 at 11 15 21

isabelroses
isabelroses previously approved these changes Aug 24, 2024
@isabelroses
Copy link
Member

image

Just noticing in the last image you sent some things appear unthemed. Is there a reason for this?

@goooseman
Copy link
Author

@isabelroses You are right, sorry for skipping that. Went through different UI elements on the page: search icon, chevron icon, scroll bar and styled them according to the style guide
Screenshot 2024-08-26 at 12 20 49

@uncenter
Copy link
Member

That bar with the tabs above it looks unthemed, as well as the background color of the details of that one book you have selected.

@goooseman
Copy link
Author

goooseman commented Sep 1, 2024

Screenshot 2024-09-01 at 10 35 12

@uncenter thx for the feedback, issues are resolved

@goooseman
Copy link
Author

@uncenter @isabelroses Hi! Is there is anything I can help with to make it merged?

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.

3 participants