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: Bookmarks #83

Merged
merged 5 commits into from
Oct 24, 2024

Conversation

Robert-MacWha
Copy link
Contributor

Frameworks PR Checklist

Changes Summary

  • Added bookmarks for pages, stored in localstorage for cross-session.
  • Added a bookmark button to the right-side UI.
  • Added a "Bookmarked" filter to the tags filter.

I ended up combining bookmarks with the tags because (a) it makes for a cleaner UI and (b) it grants more functionality - filtering bookmarked pages by their tags. I do note that it makes for a less intuitive UX (specifically, when first bookmarking an article users receive no feedback), but the hit to the UI from having a separate bookmark section seemed worse than this poor UX.

Let me know if you think it ought to be done differently, since I'm not thrilled with this implementation.

Comparison

Before After
image image
image
image

I don't like how it looks & think being able to filter by bookmarked is probably sufficient.  Slightly harder to learn, but once you do learn it's just as easy and has more features.
Updates the sidebar links when filtering by Bookmarked
Copy link

vercel bot commented Oct 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
frameworks ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 16, 2024 6:58pm

@Robert-MacWha
Copy link
Contributor Author

Also - feel free to squash and merge my code PRs. I tend to have a lot of commits while I work through things, so it can be cluttered. No need to have a messy main branch.

@Robert-MacWha Robert-MacWha mentioned this pull request Oct 23, 2024
@mattaereal
Copy link
Collaborator

I'll try to address this within this week. But in the meantime if you want to squash your commits that would be great, though I wasn't really tidy when I started working by myself haha. But now that we're open to external collaboration I'd like that to change

@Robert-MacWha
Copy link
Contributor Author

No worries — no rush on merging the changes. I need to actually get around to writing the safe harbor content (you know - the original point of me looking at this pr lol).

I don't have permissions to squash. It's an option you can select when merging the PR -
image.

Doesn't change anything, just combines all the commits into a single item in the git history.

@mattaereal mattaereal merged commit 8988adf into security-alliance:develop Oct 24, 2024
4 checks passed
mattaereal added a commit that referenced this pull request Nov 1, 2024
* add SEAL logo as favicon

* docs: Update README ##pull-requests

* fix: Fix typo

* fix: typos in pr_template

* feat: improved tag generation

* feat: Update tag display system

* Sort tags alphabetically

* Remove "Selected Articles"

* When a tag is displayed, only show articles matching the tag

* Fixed CSS on tags-dropdown

* feat: remove duplicate code & document methods

* Functionality remains, as far as I can tell, the same.  I mostly just removed a few duplicate methods related to tagsList and added documentation to make the code easier for new readers.

* refactor: move JS and CSS to seperate files

* style: Added margin around page-tags

* refactor: Extract tag initialization logic into seperate methods

* fix: announcement-stripe overflowing at lower horizontal resolutions

Also fixed layering issue due to background transparency

* feat: implemented AND selection for tags.

The code for OR selection was just commented out since I'm not sure if others actually wanted this, or if it was just me.  Can be reverted if so desired.

* feat: remove HTML styling on tag-search

Better to have all styles in the .css file

* Updating wordlist.txt and some markdown linting fixes

* Jump of line fix in README

* Updating wordlist

* Adding a few more words to wordlist.txt

* feat: Bookmarks (#83)

* feat: add bookmarked to tags filtering

* revert: remove bookmarked section from ToC

I don't like how it looks & think being able to filter by bookmarked is probably sufficient.  Slightly harder to learn, but once you do learn it's just as easy and has more features.

* fix: Fix bug where deselcting tags removes all filters

* fix: refresh sidebar when bookmarking

Updates the sidebar links when filtering by Bookmarked

---------

Co-authored-by: Mehdi Zerouali <[email protected]>
Co-authored-by: Robert MacWha <[email protected]>
Co-authored-by: Fredrik Svantes <[email protected]>
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