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

WebHost: minor css changes to make Supported Games page usable without js #2266

Merged

Conversation

black-sliver
Copy link
Member

No description provided.

@Silvris
Copy link
Collaborator

Silvris commented Oct 4, 2023

Is this different from #2259 ?

@black-sliver
Copy link
Member Author

black-sliver commented Oct 4, 2023

Yes, it's a different approach.

There is even a 3rd possible approach that is probably even cleaner from a DOM PoV: inserting the arrows and assigning the classes with JS, however that has a tendency to be blinky if the javascript is not inline in the html.

And a 4th that is inserting the arrows using css with ::before, hiding the content has the same potential blinkiness issues though.


as a side note: i would change #{{ game_name }}-arrow in JS to select .collapse-arrow instead. There is not really a reason to have an id there, but I didn't want to touch the JS.

@LegendaryLinux
Copy link
Member

I can adjust the JS to not loop over the class elements. Expect a push in a couple days.

@LegendaryLinux LegendaryLinux self-assigned this Oct 4, 2023
@KonoTyran
Copy link
Contributor

KonoTyran commented Oct 4, 2023

another alternative would be something like
https://codepen.io/Kono-Tyran/pen/BavqYYB
this uses only css to open/close. then the search feature would just need to toggle checkbox states.
page would be fully functional without js minus search.

@black-sliver
Copy link
Member Author

Oh, that's an interesting one.
I personally don't really like the raw HTML it uses (assuming HTML should be data, js and css the representation), and I think "show all" is a valid approach if the user requested a non-interactive (non-js) version of the page, but I'm sure there are uses for it.

@LegendaryLinux
Copy link
Member

TIL about subsequent combinators. I think @KonoTyran's approach here is ingenious.

If we go the route of Jinja with optional JS everywhere, this sort of thing could preserve interactivity for a lot of our pages when users have JS disabled.

I think in any case, I want to show the JS search/collapse fields, even if they are non-functional for users with JS turned off. I want to include a message like "Hey, look at the cool stuff you could have with JS turned on."

@Berserker66
Copy link
Member

It's an approach I've used myself on roll20 character sheets, as they don't allow any JS at all. Worked perfectly fine there.

@LegendaryLinux LegendaryLinux added the affects: webhost Issues/PRs that touch webhost and may need additional validation. label Oct 4, 2023
@black-sliver
Copy link
Member Author

I don't think promoting "your version" when the user asked for a "no script version" is helpful. Search functionality is still provided by ctrl+f.

Kono's suggestion on the other hand may improve usability of the page by making it faster to read through, however I don't think the old experience was all that bad.

@LegendaryLinux
Copy link
Member

@black-sliver Let me know what you think of those JS changes. The goal was to remove most id attributes and use relative element referencing.

Copy link
Member Author

@black-sliver black-sliver left a comment

Choose a reason for hiding this comment

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

I think DOM and JS looks a lot cleaner this way.
Found one mistake where it adds the class to the header instead of the content, breaking the collapse-all when clearing the search box.

WebHostLib/static/assets/supportedGames.js Outdated Show resolved Hide resolved
Copy link
Member Author

@black-sliver black-sliver left a comment

Choose a reason for hiding this comment

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

Not sure if we should leave the TODO: or how we want to tackle that in general. I've never worked with that kind of fragmented js/css where each page has its own set of files.

WebHostLib/static/assets/supportedGames.js Outdated Show resolved Hide resolved
@LegendaryLinux
Copy link
Member

Separate HTML / JS / CSS is how I've always organized things unless a framework is involved. I'm okay with removing the TODO.

@ThePhar ThePhar added the is: enhancement Issues requesting new features or pull requests implementing new features. label Oct 17, 2023
@LegendaryLinux LegendaryLinux merged commit 56796b7 into ArchipelagoMW:main Oct 20, 2023
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
…t js (ArchipelagoMW#2266)

* WebHost: minor css changes to make Supported Games page usable without js

* Update JS to use querySelectorAll, remove most id attributes from elements, use relative element selectors

* Hide content when clearing search bar

* Remove `console.log`, remove TODO

---------

Co-authored-by: Chris Wilson <[email protected]>
@black-sliver black-sliver deleted the supported-games-rework branch February 23, 2024 07:30
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
…t js (ArchipelagoMW#2266)

* WebHost: minor css changes to make Supported Games page usable without js

* Update JS to use querySelectorAll, remove most id attributes from elements, use relative element selectors

* Hide content when clearing search bar

* Remove `console.log`, remove TODO

---------

Co-authored-by: Chris Wilson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: webhost Issues/PRs that touch webhost and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants