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

Reactive UI changes #9

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

daespinozah
Copy link

I've collapsed the lower two tables into dropdowns, as well as updated the UI so it works nicer on more screen sizes.

@daespinozah
Copy link
Author

I'd like to move all those rules at the bottom to tooltips on the individual fields as a next step if this stuff looks good

@daespinozah
Copy link
Author

after the last commit I can now fully use two situations with my foldable phone open. OnePlus Open

@jmegner
Copy link
Owner

jmegner commented Feb 22, 2024

I like the accordions. I might move the avg dmg and kill chance above the accordions, OR they could become the text of the Accordion.Header elements!

I also would love to voice chat where you educate me on how to manipulate width and layout better, because I have been struggling to achieve what I want.

I think there are two big facets where we want to go in different directions.

  1. I want things horizontally squeezed together even when there is plenty of space, and I want this so 1a) your mouse/input-mechanism doesn't have to do a lot of travel to interact with elements, like modifying both shooting situations to use Px=1 and increment the BS. And 1b) it is hard to read table rows when the columns are way spaced out. Table columns should be minimal width. 1c) Same thing as 1b but for glancing between Situation 1 and Situation 2.

  2. I don't like to vertically stack two vertically-long things and require a lot of lengthy scrolling back and forth to change inputs or look at both outputs. In mobile with your changes, the two situations are vertically stacked and vertically long, thus you have to scroll up and down a lot. In mobile with what I have, you can do a quick horizontal scroll between the two situations or zoom out to see everything at once.

@daespinozah
Copy link
Author

daespinozah commented Feb 22, 2024

I think I agree with you on point one, I was trying to make maximal use of the extra space but we could shrink it a lot without it looking barren. On point two, mobile sites and apps are designed around all the information being visible horizontally with some necessary vertical scrolling. Maybe we could make a sliding pane so you can swipe between the two? My main issue originally was that all the text was too small and having to pinch to zoom and scroll around manually felt inconvenient.

@jmegner
Copy link
Owner

jmegner commented Feb 24, 2024

Note for self: admire the hover effect for the "KillChance vs Sv & W"

Some notes while we were talking...

Maybe use Carousel so mobile can swipe between situations and we have an option to go back to old way if they want to zoom out. Ah, can just have button to switch to desktop site? We don't want to have to explain how to force desktop site in the browser menu.

Some questions on if we should do the swipy based on screen size (so big tablets that don't need it won't have it), because I find that hard to correctly validate with browser screen/device emulation and I don't know all the ways to do this sort of thing. I try to put in OnePlus 10 Pro's alleged 1440x3216 resolution with ~525 ppi density, but that doesn't work when naively inputted to firefox device list. Even dividing by 2.75 (seen in other devices) didn't work. Will have to empirically figure it out? Gross. Ah, Daniel found a good site and term "viewport": https://yesviz.com/devices/oneplus-10-pro/ so I want 412x919 in firefox...confirmed. Daniel cracked the case! He also found https://viewportsizes.com/mine/ which quickly tells you your browser's current viewport size.

I still like having things compact and easy to glance between the 2 situations, rather than expanding things to use the space. You bring up a great point to use horizontal space somehow, like moving the Notes and/or a combined situation table upwards and occupying unused side space.

Possible split up of work: Daniel on Carousel/swipy thing and Jacob to experiment with moving things like Notes to unused horizontal space and maybe making a both-situation result table. Also, Jacob to do the Accordion stuff.

@daespinozah
Copy link
Author

daespinozah commented Feb 25, 2024

PC version back to smaller size, all tables now highlight row on hover (and tap on mobile). New version running on my repo!

@daespinozah
Copy link
Author

Carousel working on mobile

@jmegner
Copy link
Owner

jmegner commented Mar 1, 2024

Great work, I hope soon to have some time to do my homework.

On my phone, the delay between completing my swipe gesture and the start of the carousel transition is annoyingly long. With PC browser dev tools on your page, I changed the transition from 0.3s to 0.05s and was much happier with that. Using 0.0s made it seem like nothing was happening. What do you think about 0.05s?

Also, I'm worried about people keeping track of which situation they are on, and that we need something like "Situation X" to float/whatever-the-right-term-is at the top once you scroll down. What do you think?

@daespinozah
Copy link
Author

I was unsure about the transition time myself, it does still feel a little slow though for sure. I'll try .05s and see what that feels like. And I actually had the same thought on the situation headers! I was planning on making them stick to top of screen when you scroll up when I have some time.

@jmegner
Copy link
Owner

jmegner commented Mar 2, 2024

@daespinozah can you link me to the Accordion example where you got that accordion-related css?

@jmegner
Copy link
Owner

jmegner commented Mar 2, 2024

@daespinozah can you educate me on why you put the app in a Stack element and how that changed layout? Thanks.

@daespinozah
Copy link
Author

Here's the link for the accordion stuff:
https://react-bootstrap.netlify.app/docs/components/accordion/#accordionbutton

As for the stack element, it's a more natural fit for elements arranged vertically that's the only reason I changed it:

https://react-bootstrap.netlify.app/docs/layout/stack

@jmegner
Copy link
Owner

jmegner commented Mar 16, 2024

Thanks, but specifically where did you get the "accordion-related CSS?"

@daespinozah
Copy link
Author

The accordions in the documentation are using it. I copied from the dev tools window.

@jmegner
Copy link
Owner

jmegner commented Mar 16, 2024

Ah, thanks.

@jmegner
Copy link
Owner

jmegner commented Mar 17, 2024

91f33ec incorporates Accordions based on your work. I will look at the SwipeableViews stuff next. Thanks again, and critique of what I did is very welcome.

@daespinozah
Copy link
Author

I really like that you put the most relevant data in the header of the accordions, looks good!

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