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

Lists UI #1720

Merged
merged 39 commits into from
Jan 3, 2025
Merged

Lists UI #1720

merged 39 commits into from
Jan 3, 2025

Conversation

bryanmontz
Copy link
Contributor

Merges lists UI feature branch.

bryanmontz and others added 30 commits December 23, 2024 08:51
Allow Github actions to update provisioning profiles
added feed source customizer drop-down view #102
add empty state for lists/relays drop-down
added support for decrypting private tags in kind 30000 lists
added pop-up tip for feed customization #101
added remembering which feed source is selected
update to custom styled segmented picker
fixed a case where lists don't show up immediately after signing in
fixed a minor cell layout issue on feed customizer drop-down view
@bryanmontz bryanmontz self-assigned this Jan 2, 2025
@bryanmontz bryanmontz marked this pull request as draft January 2, 2025 14:38
@mplorentz
Copy link
Member

👀

Copy link
Member

@mplorentz mplorentz left a comment

Choose a reason for hiding this comment

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

I reviewed most of this but I didn't have time to look at the View code. I had a lot of trouble testing it because Listr wasn't working and when I did finally get some lists going Nos wouldn't display any of them. I started putting breakpoints in to see what was happening and then it started working 🤦‍♂️ I can't reproduce it anymore. My hope right now is that my local Core Data got into a weird state from testing this and Josh's list work before it was finished.

Leaving some minor comments below. Overall I'm not against merging this if it's working for others.

.github/workflows/testflight-staging-deploy.yml Outdated Show resolved Hide resolved
Nos/Controller/FeedController.swift Outdated Show resolved Hide resolved
Nos/Controller/FeedController.swift Outdated Show resolved Hide resolved
Nos/Service/CurrentUser.swift Show resolved Hide resolved
@mplorentz
Copy link
Member

Screenshot 2025-01-02 at 9 38 05 AM

I think we should bump the "new notes available" toast down by the height of the new feed selector bar.

@mplorentz
Copy link
Member

mplorentz commented Jan 3, 2025

I ran across a bug with displaying a list on launch. My steps to reproduce:

  1. Select a list
  2. Force quit the app
  3. Relaunch
    Expected: my list is still selected and content from it is displayed
    Actual: my list is still selected but a note from someone not on the list is displayed. If I scroll down and back up it disappears.

(this video starts at step 3)

ScreenRecording.mp4

Github is failing to render the video for me, but I was able to right click it and "save as" so hopefully you can too.

@bryanmontz bryanmontz marked this pull request as ready for review January 3, 2025 13:12
@bryanmontz
Copy link
Contributor Author

@mplorentz I think/hope I fixed the issue you mentioned above with a hacky delay. I want to return to this after we ship.

@mplorentz
Copy link
Member

Unfortunately the hack didn't fix the feed when launching with something other than "Following" selected. I'm going to comment out the code that loads the last selected feed on app launch, because that will feel less broken. We can get it fixed up in a future PR.

Other than that, this is working great for me!

@mplorentz mplorentz enabled auto-merge January 3, 2025 16:54
@mplorentz mplorentz added this pull request to the merge queue Jan 3, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jan 3, 2025
@mplorentz mplorentz enabled auto-merge January 3, 2025 19:45
@mplorentz mplorentz disabled auto-merge January 3, 2025 19:45
@mplorentz mplorentz merged commit b97acbf into main Jan 3, 2025
3 of 4 checks passed
@mplorentz mplorentz deleted the feature/lists-ui branch January 3, 2025 19:45
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