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

Add config option to use grid for global layout #2516

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

liu-ziyang
Copy link
Contributor

@liu-ziyang liu-ziyang commented Oct 28, 2023

This PR adds a configuration that turns the entire app into a giant grid that allows moving and resizing components to use the full screen

Configuration button:

Screenshot 2024-03-10 at 11 40 34 PM

Customizable layout:
Screenshot 2024-03-10 at 11 39 22 PM

@liu-ziyang
Copy link
Contributor Author

Update: Had a really busy week and didn't make much progress. (Also need to grind the fish event). Will pick up next week on reworking the config layout. I think the grid layout should be with window/overlay layout buttons instead of the tab area buttons. gonna correct that and then move onto testing the canvas wrapper

@KochiyaOcean
Copy link
Member

The main problem should be the webview area should has a fixed ratio and can be set to use both a fixed value or controlled by window size

@liu-ziyang
Copy link
Contributor Author

The main problem should be the webview area should has a fixed ratio and can be set to use both a fixed value or controlled by window size

I am thinking we can lock the webview area size to be not adjustable within the grid to simplify the logic, basicly only allowing user to move the webview area around but not allowing resizing it. Need to explore whether this is possible though (I don't know the code base or react well enough to make recommendations.)

@liu-ziyang
Copy link
Contributor Author

liu-ziyang commented Nov 11, 2023

@KochiyaOcean Hi, after I updated the branch with the latest the display settings are gone. Any idea why this would happen? Looking at my code changes I don't think it's from me..

Screenshot 2023-11-11 at 11 23 51 AM

rebased to 10.9.2 it works as expected:
Screenshot 2023-11-11 at 11 27 21 AM

@liu-ziyang
Copy link
Contributor Author

I tried to wrap the main components as grid item in the poi-app.es but couldn't quite iterate because the whole app breaks. I think the underlying component (KanGameWrapper I think) is expecting something from its parent? I will keep trying but would really appreciate some inputs here

@KochiyaOcean
Copy link
Member

@liu-ziyang Try to update to the latest electron version, I think there is a related patch.

@liu-ziyang
Copy link
Contributor Author

Update: I haven't give up yet! 😄

Right now I am learning react and nextjs on my spare time, hoping it would help me understand how javascript works better so I eventually understand what I need to do here, so it is going to take some time for me to come back to this PR.

I should be able to spend some time around end of December and dive a bit deeper on what is needed here

@liu-ziyang
Copy link
Contributor Author

Made some progress on the grid, but at the same time the game size impacted the grid layout numbers, will think about how to handle this next
Screenshot 2023-12-12 at 6 11 53 AM

@liu-ziyang
Copy link
Contributor Author

Still exploring the layout, I think what I may end up doing is dropping the overview panel in the grid mode and move everything to the grid while allowing toggling between settings and fleet panel. Any suggestions welcomed

@liu-ziyang
Copy link
Contributor Author

@KochiyaOcean are you free to do a quick call any time you are available? I am kinda stuck trying to figure out how to wrap the settings panel in the grid. Would really appreciate if you have any tips on how the element boundary/sizing works

Screenshot 2024-01-03 at 9 03 58 PM

@KochiyaOcean
Copy link
Member

@liu-ziyang maybe you need to show your code changes

views/app.es Outdated
<KanGameWrapper key="frame" />
</div>
</ResponsiveReactGridLayout>
<PoiApp />
Copy link
Contributor Author

@liu-ziyang liu-ziyang Jan 4, 2024

Choose a reason for hiding this comment

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

@KochiyaOcean There are 2 approach I am trying, the first one is leaving the poi app outside the grid, which resulted in the control panel taking over the whole screen and I can't figure out how to resize it.

The other approach is put this as a div like other components, I personally prefer doing so because it would then allow moving the control panel around. but if I do that, somehow the control will completely disappear even though the grid item is there and draggable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2024-01-03 at 9 23 52 PM

when the poi app is wrapped in the grid

Copy link
Member

Choose a reason for hiding this comment

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

I think it may caused by PoiApp component is actually a fragment and some event bundling failed to work. Maybe you can check the console to find if there is any js wise errors. In the other way, if the DOM compoents are mounted correctly, it would be more likely to be a css issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I managed to get devtools to show up again but couldn't figure out what event bundling is broken, there are many error messages and don't seem to be relevant to my untrained eye. any theory based on file change in this PR?

@liu-ziyang
Copy link
Contributor Author

I changed how the HTML tags wrapped and made some progress by keeping the hierarchy of poi-main and poiApp, there must be some bundling somewhere I can't find that relies on this particular arrangement.

<ResponsiveReactGridLayout>
  <div><div> // other items
  <div>
    <poi-main>
      <PoiApp />
    </poi-main>
  </div>
<ResponsiveReactGridLayout>

^ seems to be working

That said, the event connection may still be having some issue as I am seeing empty fleet and setting information, I will keep digging but any tips appreciated. (also I have not been able to bring up the devtools for a while, I am not sure why or what I did that broke it)
Screenshot 2024-01-06 at 1 02 15 PM

@liu-ziyang
Copy link
Contributor Author

Addicted to palworld and didn't have a chance to make much progress recently.

if anyone is interested feel free to take over, otherwise I will likely revisit the event bundling issue sometime in Feb after my work settles down a bit

@liu-ziyang
Copy link
Contributor Author

Sorry for the inactivity here, was busy with interviews and job change...

I managed to isolate the event issue, the error seem to suggest the plugins are blocking the interface from working:

create-store.es:85 Warning: You provided a `checked` prop to a form field without an `onChange` handler. This will render a read-only field. If the field should be mutable use `defaultChecked`. Otherwise, set either `onChange` or `readOnly`.
    at input
    at label
    at /Users/ziyang/Projects/poi/node_modules/@blueprintjs/core/lib/cjs/components/forms/controls.js:30:32
    at Blueprint5.Radio
    at div
    at I (/Users/ziyang/Projects/poi/node_modules/styled-components/dist/styled-components.cjs.js:1:22844)
    at RadioCheck (/Users/ziyang/Library/Application Support/poi/plugins/node_modules/poi-plugin-prophet/src/views/settings-class/radio-check.js:46:5)

Not sure what exactly is this error message complaining about. I am assuming my change broke some assumptions the plugins are making?

@liu-ziyang
Copy link
Contributor Author

@KochiyaOcean
I think I have the rusty prototype ready for preliminary review and maybe trying out.

Some of the major area to work on next I think is:

  • polish the code in the app.es, right now they are mostly copied over without much consideration
  • testing switching between display modes
  • default to ships view when on grid mode, even better if we can hide the main panel
  • fine tuning UI looks, the configurations and setup is very silly and hardcoded, I don't quite know how can I make it more automatic via css etc
  • cross OS support (not sure what I break in the process of changing code)
Screenshot 2024-02-23 at 9 54 57 PM

@liu-ziyang
Copy link
Contributor Author

since the early spring event is upcoming, likely I will be away doing that before coming back again, hopefully I can finish polishing up before end of March if the event is not too hard

@liu-ziyang liu-ziyang requested a review from KochiyaOcean March 11, 2024 03:33
@liu-ziyang liu-ziyang marked this pull request as ready for review March 11, 2024 03:33
@liu-ziyang liu-ziyang changed the title WIP: Explore react grid layout Add config option to use grid for global layout Mar 19, 2024
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