osmium icon indicating copy to clipboard operation
osmium copied to clipboard

Search rows

Open jboning opened this issue 10 years ago • 7 comments

I've finally got this in a state where I think it's worth considering. Here's what it looks like at the moment:

https://www.dropbox.com/s/y6rgugchx4blws3/Screenshot%202014-02-26%2003.36.29.png https://www.dropbox.com/s/qz4fnoqnw17wwtk/Screenshot%202014-02-26%2003.36.52.png https://www.dropbox.com/s/s6gjjmqa1nrqcpj/Screenshot%202014-02-26%2003.37.10.png

Feedback welcome!

jboning avatar Feb 26 '14 11:02 jboning

I just realized I never fixed the corp/alliance/personal visibility indicators to work in the new layout. I will get to this tomorrow or something. Still, take a look :)

jboning avatar Feb 26 '14 11:02 jboning

A couple of remarks:

  • Please don't put useless commits in the history, like 7b121f3. You can rewrite history and trash them before making the pull request.
  • Don't put colors in main sass files, colors go in themes/dark.scss and themes/light.scss. Also use hsl() instead of rgb(), it makes it easier to finetune them by hand.
  • I'm sure people will complain that they want the old layout back (it has its benefits), having the choice between grid/table layout in search results would be nice.
  • Also, default skillset stuff was properly implemented in staging, see https://github.com/Artefact2/osmium/commit/9a226d9bb95dce9999c96504465e7029337c6d19#diff-12e55340bd26b6028d53601477fef6d9R1245
  • <table start= is invalid HTML.
  • Always explicitely use <tbody> (and optionally <thead>/<tfoot>) in tables, this is to make things less confusing in XHTML5.
  • Vendor-specific CSS properties should go before the standard property, for future compatibility (this is minor).
  • Last (and not least!), don't rely /only/ on color for conveying information (flyability in your case). This is bad accessibility. See GUIDELINES.md.

Artefact2 avatar Feb 26 '14 13:02 Artefact2

Bullet by bullet:

  • Sorry about the trashy history. I can fix it and push -f if you want :)
  • Since the colors are semitransparent, they actually work okay the light theme too: https://www.dropbox.com/s/5avukxpm0ha4um4/Screenshot%202014-02-26%2009.13.57.png But if you want to be strict about this for code style's sake, I'm fine with changing that. (I guess without a skillset to check flyability it's not so great: https://www.dropbox.com/s/xhak24m0qq5w2si/Screenshot%202014-02-26%2009.17.14.png Also now I notice that ship names don't show up. Needs some work.)
  • Hmmm. I know it's slightly less information dense (worse comparatively as the screen gets wider), but I feel pretty strongly that the information is easier to consume this way. The design could probably use some tweaks, but I'd like to hear what you think the major benefits are to the old way (not being snarky here--I actually do!).
  • Whoops, guess I've got my head under a rock. I will merge staging and update this to use that when I've got a chance.
  • Whoops. No excuse for this one; must've left it from the old list tag. Will fix.
  • Whoops, bad at (x)html, will fix.
  • Whoops, bad at (s)css, will fix.
  • Finally, a point I can push back on! If you can't fly a fit, I included a section below the fit name showing number of SP until being able to sit in the hull (if relevant) and until being able to fly the whole fit. So the colors are indeed redundant :)

I should be able to get to the various "will fix" things tonight (or I guess that's tomorrow morning for you).

jboning avatar Feb 26 '14 17:02 jboning

P.S. If you take nothing else from this, please please take the ship names on images: https://www.dropbox.com/s/7u6iswce14s8rmr/Screenshot%202014-02-26%2009.47.09.png That will work just as well outside the row design, and is a huge usability improvement. I'll probably break that out into a separate and hopefully less contentious pull request tonight as well.

jboning avatar Feb 26 '14 17:02 jboning

There's no rush, I will likely not merge this until I am done with porting stuff to DOM, and that is a lot of work. So you still have some time to polish things up.

I'm actually not a fan of the background color covering the entire row, in my (humble) opinion it feels very intrusive and eye-catching (too much perhaps). Maybe a skill icon with a colored background, or using a background-color for only one <td> would be better suited.

About the grid vs table, they both have their ups and downs, but Osmium used to have a table layout that looked like yours a long time ago (see some old screenshots of version 0.6.0). People didn't really like it. Also, the space difference is all but small with 1920x1080.

Artefact2 avatar Feb 26 '14 18:02 Artefact2

Yeah, the flyability styling could definitely stand to be played around with, I certainly haven't explored all of the design space for that. I'd argue in favor of it being very noticeable, though. As a player looking at a list of fits, the top thing affecting whether a result is useful to me is whether I can fly it.

Good to know about the historical context on tables vs grid. Looking at those screenshots, I think the current grid has a couple of big advantages over the table:

  • Shows useful information. If I am glancing over a bunch of results, the top things I care about are whether I can use the fit and what its base stats are. (Aside: showing reps instead of dps for logi would be cool; working cap stability in would also be cool.) Votes, tags, author are a mixture of "maybe I'll glance at that" and "might be useful if voting/reputation systems gain momentum".
  • Doesn't waste so much screen real estate; the 0.6.0 table literally uses the left half of the screen.

I think the "useful information" complaint about the old table doesn't apply to this one, since it brings over the information from the grid. The screen real estate complaint might still be valid. At least it's not quite so lopsided as the old one was :) I'm not sure that's enough though.

Were there other complaints about the old table that I'm missing?

I'm now thinking I should break the skill display stuff and the table stuff into separate changes as well. This really did roll a lot of things together that don't need to be

jboning avatar Feb 26 '14 20:02 jboning

Commit e0bb15b should be interesting for you. I've finally moved the awful formatting code in inc/search.php to DOM.

Here's how I would implement a tabular layout:

  • Make render functions in dom-format-loadout.php similar to formatLoadoutGridLayout and makeLoadoutGridLayout that deal with tables. You can probably refactor most of the code in makeLoadoutGridLayout too.
  • In \Osmium\Search\make_pretty_results, also generate a simple way to choose between the grid and the table layout (I'm thinking about two small-ish self-explanatory icons, but pick whatever you think is best), then use it to determine whether to call makeLoadoutGridLayout or the other one that outputs a table.

Now, formatLoadoutGridLayout will cache its result, so obviously you can't generate skill info for the current character right away and put that in the shared cache. There's probably multiple ways of doing it, but I've though about this one:

  • In formatLoadoutGridLayout, generate a custom element with the minimum skill prerequisites of fitted types (for example, <o-prerequisites :1="typeid1,typeid2,..." :2="typeid1,..." /> etc. Since this is not character-specific, this is safe to cache.
  • In the function rendering o-prerequisites, use whatever skillset should be used and alter the DOM to reflect skill prerequisites. This is a fairly lightweight operation (we don't even need to fetch the loadout with get_fit()).

Anyway, I'm sorry for messing up most of your code this late, but if you have a simpler idea feel free to experiment and let me know!

Artefact2 avatar Feb 28 '14 00:02 Artefact2