AllYourBot / hostedgpt

An open version of ChatGPT you can host anywhere or run locally.
MIT License
303 stars 122 forks source link

PWA Anchor Improvements #388

Closed jasonpaulso closed 1 month ago

jasonpaulso commented 1 month ago

Fixes #298 (partial)

note: changes to gemfile, develepoment.rb, and _head are minor auto-formatting changes made to files I'd had opened, no functional changes otherwise.

krschacht commented 1 month ago

@jasonpaulso after I merged in your other branch, I noticed that this branch had a merge conflict. I came over here and merged main in, in order to resolve the conflict, and after doing that the diff is basically showing no changes.

Is it possible that all the changes you had in this branch were already in the other branch? I'll note that the description:

nav links should navigate on one tap and no longer show flyout on long press

This appears to be working for me in main. Nav links definitely are working on one press and a long press causes a tiny flyover thing but it's not really a flyover, it looks almost like a copy paste dialog. I think we can consider this issue fixed!

Let me know if I actually messed up this branch of yours :) and if I can help further. I looked briefly through the commit history but I wasn't sure which commits were the material ones and I think these features are already fixed in main.

jasonpaulso commented 1 month ago

TBH at this point I have no clue what exactly what happened here. To be clear, you do feel like these change have done the trick, insofar as fixing the items in question? On Jun 10, 2024, at 5:21 PM, Keith Schacht @.***> wrote: @jasonpaulso after I merged in your other branch, I noticed that this branch had a merge conflict. I came over here and merged main in, in order to resolve the conflict, and after doing that the diff is basically showing no changes. Is it possible that all the changes you had in this branch were already in the other branch? I'll note that the description:

nav links should navigate on one tap and no longer show flyout on long press

This appears to be working for me in main. Nav links definitely are working on one press and a long press causes a tiny flyover thing but it's not really a flyover, it looks almost like a copy paste dialog. I think we can consider this issue fixed! Let me know if I actually messed up this branch of yours :) and if I can help further. I looked briefly through the commit history but I wasn't sure which commits were the material ones and I'm thinking we're good.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

krschacht commented 1 month ago

@jasonpaulso Yes, I think so! I think you got all three of those fixed.

I just looked at the #298 list and updated it. There are a couple items on there which are primarily visual and I think we should defer those b/c OpenAI has updated their own design and soon I want to catch up the app to the new design. For example, increase the tap targets. That's an example of one to defer.

I just went through the list and marked those and pushed them down.

But I think the top 4 on that list are still worth doing. The first item is probably the most janky thing to fix, but that one might be really hard. I'm not sure. The second item will also be a little tricky but will definitely be worth doing because most people don't know how to create a PWA from a web page. The 3rd and 4th item might be really quick.

krschacht commented 1 month ago

You're awesome for helping with so much! The visual design of the app is so tight and so much nicer than I was achieving on my own. Each of these issues can be so tricky to figure out!

jasonpaulso commented 1 month ago

Awesome!

Re: the next priority tasks;

This is definitely, at least mostly, doable. In fact, before the Memorial Day holiday I spent a few days in the weeds on this—nearly getting it perfect, plus a few more nice to haves I was approaching (I’d really like to slide in the menu with a swipe from the left…) Ultimately I chickened out when I emerged seeing the extensive reorganization of components it ultimately required—and what you might think upon seeing that PR. So, I’d say your assessment is correct, we can probably eliminate some/most of the jank—if you are good with my making a few assumptions about how the f/e can be refactored to achieve a more app-like experience. I think this is an interesting one and I can’t say how challenging it would be—it’s been a few years, but I’ve definitely been asked to look into a few things like this by product and marketing designers more than once and ultimately there was never a solid-enough solution. I would say we can confidently introduce some indication that the experience would be better if the user installs the PWA. Speaking of which, I think some kind of caching would be great to consider, so I can at least read my previous conversations while I’m in the subway… 😅
This is an interesting one. I’ll look into what can be done. I’ve also tinkered with this issue a bit a few weeks back. It involves the “theme_color” meta property. There is an option to set a theme color for dark and light mode, but it is only respects the “system” setting. I’d started to work on adding in a bit of logic to dynamically set the color when the app rendered. I can revisit this as well.

On Jun 11, 2024, at 10:42 AM, Keith Schacht @.***> wrote:

@jasonpaulso https://github.com/jasonpaulso Yes, I think so! I think you got all three of those fixed.

I just looked at the #298 https://github.com/AllYourBot/hostedgpt/issues/298 list and updated it. There are a couple items on there which are primarily visual and I think we should defer those b/c OpenAI has updated their own design and soon I want to catch up the app to the new design. For example, increase the tap targets. That's an example of one to defer.

I just went through the list and marked those and pushed them down.

But I think the top 4 on that list are still worth doing. The first item is probably the most janky thing to fix, but that one might be really hard. I'm not sure. The second item will also be a little tricky but will definitely be worth doing because most people don't know how to create a PWA from a web page. The 3rd and 4th item might be really quick.

— Reply to this email directly, view it on GitHub https://github.com/AllYourBot/hostedgpt/pull/388#issuecomment-2160942536, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACT5ZUWSV24W55U652CBRPLZG4EDXAVCNFSM6AAAAABI3GGID6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRQHE2DENJTGY. You are receiving this because you were mentioned.

krschacht commented 1 month ago

On the first one, I wouldn't shy away from significant component re-organization, but one way I find is really helpful is: instead of tackling that whole component re-organization, instead build a toy demo experience. You could add a new demo.html file to the repo (or whatever you need it to be) which has blocks/wireframes of all the major elements. This is both a proof of concept and also the key reference as we figure out how to incrementally migrate the app towards that new structure in a few different commits. And once we have a reference, then I can better help with some of the pieces that we need to land.

I firmly believe there is a better overall organization of the front-end code / components / partials / CSS but I haven't had a clear enough vision yet of what to move towards.

Good thoughts on all the others!