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

update readme and begin image restructuring #123

Merged
merged 12 commits into from
Mar 13, 2023

Conversation

tjex
Copy link
Contributor

@tjex tjex commented Mar 8, 2023

Refers to #120

Added some instructions to the readme, which include personal design considerations (highlighting with the transparency box and orange/yellow colours otherwise to work better with Actual's color scheme) and a draft of communicating the structure ideas outline below.
Please contest any of it.

Also updated about the @2x.png scheme from #110

Was just thinking to show this preliminary work to check it's in an agreeable direction or not before continuing.

The changes in this PR boil down to (and looking for feedback on):

  1. locking in the idea of reusable images placed in their own subdirectorie: eg static/img/elements/sidebar/
  2. aligning on some design considerations (highlight tactic / color scheme) and how to communicate them in the readme
  3. aligning on a strategy to organise other images (elaborated on below)

For point 3 there are two options I think.

3a. the images for the page at github.io/docs/Budgeting/filters, would be stored in static/img/filters
3b. the images for the same page, would be stored in static/img/budgeting

Option 3a - ie. one folder per page at static/img/:

  • will end up creating many more folders in the img directory.
  • allows the page itself to be moved to a different section of the webpage, and it's image paths to keep making sense.
  • helps to maintain mental context as to which images are related to which pages, making it much easier to work with when writing the page
  • allows for minimal image naming conflicts
  • care will need to be given to name the folder and .md file the same, to keep things understandable later down the track
  • if a page gets deleted, deleting its images folder will break any pages that also use those images (but this new structural idea is designed with the idea of that not needing to happen often, if at all. ie a folder for commonly reusable images `static/img/elements/...).

Option 3b - ie. one folder per subsection at static/img/:

  • images can be more easily manually searched through in a folder (as all images for all pages under Budgeting would be in this folder)
  • less thought needs to be given to saving and managing images
  • higher likelihood of ending up with redundant images left in the folder after page deletion / change
  • if a page moves to a different subsection, either image paths in the page.md will need to be updated and images moved to another subsection, which can potentially break other linkings in pages from the same subsection that use the same images. Or the image paths stay unchanged, breaking the relationship between page position in repo structure and where the images are found.

@netlify
Copy link

netlify bot commented Mar 8, 2023

Deploy Preview for zingy-bonbon-b1b286 ready!

Name Link
🔨 Latest commit 8be9299
🔍 Latest deploy log https://app.netlify.com/sites/zingy-bonbon-b1b286/deploys/640e2f7d0af0800009e543fa
😎 Deploy Preview https://deploy-preview-123--zingy-bonbon-b1b286.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

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

I like this approach! A couple of comments below plus one more: how do you feel about encouraging people to use the built-in test budget for screenshots? I don’t know whether it would be consistent enough in places like the sidebar because it has some inherent randomness so maybe as an alternative you/we could make a demo budget and save it to this repo for testing purposes?

README.md Outdated
- click the annotate icon
- from the 'shape' selector, select the 'transparency' tool

![](/static/img/repo/highlighting.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

Files intended to be viewed on GitHub should use relative paths:

Suggested change
![](/static/img/repo/highlighting.png)
![](./static/img/repo/highlighting.png)

Copy link
Contributor

Choose a reason for hiding this comment

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

If we are using images in the readme can these come out of the /docs/ folder altogether please if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't understand what you mean by "come out of the /docs/ folder altogether".
Where should images be saved that are destined for the repo readme?
This was a screenshot taken specifically for the readme and is not an image used in actual docs.

README.md Outdated Show resolved Hide resolved
sidebars.js Show resolved Hide resolved
README.md Outdated
- click the annotate icon
- from the 'shape' selector, select the 'transparency' tool

![](/static/img/repo/highlighting.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are using images in the readme can these come out of the /docs/ folder altogether please if possible.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@tjex
Copy link
Contributor Author

tjex commented Mar 9, 2023

Thanks both for the detailed feedback!

alternative you/we could make a demo budget and save it to this repo for testing purposes?

I was also considering this idea and agree. It would reduce friction for people to get involved if they can just import a demo budget with transactions, accounts, etc already set up.

It will also create some continuity throughout the docs due to reusing the same account names / payees / etc.

I can make an issue for it.

@tjex tjex requested review from rich-howell and j-f1 and removed request for rich-howell and j-f1 March 12, 2023 17:13
Copy link
Contributor

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

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

Nice! I like this approach. Happy to merge now or wait if you still want to do more in this PR.

@tjex
Copy link
Contributor Author

tjex commented Mar 12, 2023

Nice! I like this approach. Happy to merge now or wait if you still want to do more in this PR.

great. well I'm aiming to reorganise every image that's so far being used.
I should have some good time in the coming week to do this. So will just ping you when either it's all done, or I have to drop it for a bit. In which case it would be good to merge, so that others contributing won't create more images to restructure.

@tjex
Copy link
Contributor Author

tjex commented Mar 12, 2023

hey @j-f1 . Smashed it all out now 🤓

There were a bunch of unused images leftover. I moved them into img/unused. Nothing broke, but didn't want to just delete as you guys have a better handle on what images may still be in need / etc. So will let someone else deal with that.

But yeah, looking cleaner now and easier to keep a track of :~)

Worth double checking that 'Open Issues' is an appropriate name for the issues link in the sidebar.
And I'm still not 100% sure what @rich-howell meant by images in the readme coming from /docs/?

But if it's nothing but a thing, then I'm not worried. Just let me know if anything should be changed 👍

@j-f1
Copy link
Contributor

j-f1 commented Mar 12, 2023

I believe actual.png and favicon.png are used to render the website header and favicon and should be moved back to their old locations but the other unused files can be deleted (we can always get them back from git history if needed).

@tjex
Copy link
Contributor Author

tjex commented Mar 12, 2023

ah oops. thought it was just the .ico and .svg.
done.

@tjex
Copy link
Contributor Author

tjex commented Mar 12, 2023

spotted one other mistake now too. will have another look through and re-request.

@tjex
Copy link
Contributor Author

tjex commented Mar 12, 2023

now should be good @j-f1

Copy link
Contributor

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

@j-f1 j-f1 merged commit cf0c17a into actualbudget:master Mar 13, 2023
@tjex tjex deleted the 120-img-structure branch March 13, 2023 08:52
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