swag-for-dev icon indicating copy to clipboard operation
swag-for-dev copied to clipboard

Performance + UI/X Improvements

Open vikaspotluri123 opened this issue 7 years ago • 14 comments
trafficstars

I'm in the progress of implementing performance improvements both to the backend and frontend. Here's what's being done:

  • [x] minify html at buildtime
  • [x] use gulp@next vs github:gulp#4
  • [x] load scripts at document end vs head
  • [x] remove swag-img:build-data task (processing is done on gulpfile load)
  • [x] Normalize formatting + function declarations
  • [x] Add cachebusting + extended caching
  • [x] Add .editorconfig to normalize formatting

  • [x] filter: toggle visibility based on classes to reduce re-rendering requirement
  • [x] sort: Clone DOM nodes rather than use template literals. This is to DRY up and possibly get performance improvements
  • [x] Disallow sorting by difficulty when only one difficulty is shown
  • [ ] ~Define minimum image height (currently thinking 275px) to prevent content jumping~
  • [x] Hide filters <noscript>

Feel free to add any other suggestions, or discuss anything above

  • [ ] Move tags to the same row as swag
  • [ ] Rename all difficulties to all and add the word difficulties after the input
  • [ ] Add a no items found item to be shown when filtering hides everything (I was able to trigger this with easy difficulties that are tagged device

#108

vikaspotluri123 avatar Oct 07 '18 06:10 vikaspotluri123

These points are great! I believe, and this is just my opinion, that we use a proper web framework like Express. It might seem overkill, but IMHO it will do most justice to the optimisations @vikaspotluri123 mentioned. Moreover, using Gulp to help pre-render data, optimise it, and build the files seems alright, but we can get the same things with much less code and more accessible. Also, URL handling will be made much simpler using simple routes. Thoughts?

plibither8 avatar Oct 07 '18 06:10 plibither8

Since the optimizations I'm making are to the gulp build process, express would technically negate them 😛 That's not to say express is a bad idea though 😄

The improvements I'm making will hopefully make the code more readable, albeit slightly bigger in size (let's just say it's stockpiling for winter 😉) and with the current spec I have in mind, will remove the need for the swag in a JSON format.

I personally prefer static over dynamic (ironically I prefer backend over frontend 🤫) because it cuts out a vector for vulnerability, whether it be from contributor-introduced bugs or an attack vector, and it reduces maintenance costs overall

vikaspotluri123 avatar Oct 07 '18 06:10 vikaspotluri123

You do make some good points, though I'd still be implicitly pushing towards a web framework (don't mind me :smile:).

and with the current spec I have in mind, will remove the need for the swag in a JSON format.

This sounds interesting, could you elaborate?

plibither8 avatar Oct 07 '18 06:10 plibither8

This sounds interesting, could you elaborate?

Currently everytime an input changes, swag has to be rerendered. That doesn't have to happen since we can use native javascript dom manipulation to show / hide swag items. The most taxing input change is sorting, as there really isn't an easy way to rearrange DOM nodes natively (I think part of the reason might be the UI vomit implications for that are high). StackOverflow has some discussion to do this, and I'm going to experiment with relative performance to decide if it's worth rerendering swag in the special case of sorting

vikaspotluri123 avatar Oct 07 '18 07:10 vikaspotluri123

Ah, I misinterpreted it as removing the need of data.json. My bad :sweat_smile:

plibither8 avatar Oct 07 '18 07:10 plibither8

Only on the client side 😉

vikaspotluri123 avatar Oct 07 '18 07:10 vikaspotluri123

there really isn't an easy way to rearrange DOM nodes natively

Not with JS, but with CSS there is: use flexbox and the order property! A good demo on CSS-Tricks: https://css-tricks.com/almanac/properties/o/order/

plibither8 avatar Oct 07 '18 07:10 plibither8

@vikaspotluri123 Should I try working on:

filter: toggle visibility based on classes to reduce re-rendering requirement sort: Clone DOM nodes rather than use template literals. This is to DRY up and possibly get performance improvements Disallow sorting by difficulty when only one difficulty is shown

or are you handling that?

plibither8 avatar Oct 07 '18 10:10 plibither8

Not with JS, but with CSS there is: use flexbox and the order property!

Using the order property doesn't update the a11y tree, so it's not suggested to use it for major changes like this 😬

@vikaspotluri123 Should I try working on:

I have some work done, but feel free to take over if you want

vikaspotluri123 avatar Oct 07 '18 15:10 vikaspotluri123

Cachebusting & extended caching has been added by #93. Does it lacks anything?

aslafy-z avatar Nov 06 '18 15:11 aslafy-z

Not that I am aware, everything seems to be working, correct?

vikaspotluri123 avatar Nov 06 '18 15:11 vikaspotluri123

Looks good to me, yes. It only needs a rebase.

aslafy-z avatar Nov 06 '18 16:11 aslafy-z

Let's bump that one. There are still some points to PR.

aslafy-z avatar Oct 09 '19 17:10 aslafy-z

Related to https://github.com/swapagarwal/swag-for-dev/issues/407

aslafy-z avatar Oct 09 '19 17:10 aslafy-z