js: Namespaces#index refactoring w/ Vue #1371

vitoravelino · 2017-08-08T11:27:14Z

Well, I wasn't expecting a long PR but... \_("/)_/

Here are some of the changes:

Created custom elements for namespaces (namespace-panel, new-namespace-form, namespace-table, namespace-table-row, namespace-visibility)
Added sorting columns (similar to Allow sorting of tags depending on the client side #590)
Fixed pagination (similar to The pagination only shows up after a complete reload of the page. #585)
Improved UX of changing namespace visibility (see gif below)
Updated slim gem
Updated active_model_serializers gem
Used serializers (check namespace_serializer.rb) for json rendering
Added moment for dates (kind of overkill but a good thing in the long term)
Added lodash and lodash babel plugin (makes partial imports possible)
...

If we agree on going with Vue all the way, I'll start working on the missing checklist below. Let me know if I should proceed with it or not.

Missing

~~Make sorting and pagination reachable through URL (e.g.: orderBy=clean_name&sortAsc=true)~~ Make namespace table sorting reachable through URL #1373
~~Use <namespaces-panel> on Team#show page~~ Reuse <namespaces-panel> component on Team#show page #1374
~~Remove legacy components (new-namespace-form and namespaces-panel)~~ Reuse <namespaces-panel> component on Team#show page #1374
~~Add global pagination config~~ Add global pagination limit config value #1375
~~Inline validation with bootstrap css classes (e.g.: http://1000hz.github.io/bootstrap-validator/#validator-examples)~~ Add inline validation for fields in Namespaces#index form #1376
Fix Created at column format
Fix tests
Tests for the new format.json responses

vitoravelino · 2017-08-08T13:19:59Z

Gemfile.lock

+    active_model_serializers (0.10.6)
+      actionpack (>= 4.1, < 6)
+      activemodel (>= 4.1, < 6)
+      case_transform (>= 0.2)


I couldn't find a package for this. :(

vitoravelino · 2017-08-08T13:20:08Z

Gemfile.lock

+      actionpack (>= 4.1, < 6)
+      activemodel (>= 4.1, < 6)
+      case_transform (>= 0.2)
+      jsonapi-renderer (>= 0.1.1.beta1, < 0.2)


I couldn't find a package for this either. :(

vitoravelino · 2017-08-08T13:20:43Z

Gemfile.lock

@@ -20,8 +20,11 @@ GEM
      erubis (~> 2.7.0)
      rails-dom-testing (~> 1.0, >= 1.0.5)
      rails-html-sanitizer (~> 1.0, >= 1.0.3)
-    active_model_serializers (0.9.0)
-      activemodel (>= 3.2)
+    active_model_serializers (0.10.6)


Upgraded because 0.9 was buggy

vitoravelino · 2017-08-10T10:50:05Z

@mssola ready for review! 😄

mssola · 2017-08-14T08:27:36Z

I just merged #1299, I think this affects this PR.

vitoravelino · 2017-08-14T11:57:58Z

@mssola Rebased!

vitoravelino · 2017-08-14T12:01:25Z

@mssola Are we going with Grape or are we still waiting @jordimassaguerpla packaging feedback on it?
Because If we won't revert that, I believe I could already take advantage of it on this PR instead of going with the standard rails controllers. What are your thoughts?

mssola · 2017-08-14T12:28:59Z

@vitoravelino I've already merged it, so it's safe to say that we are at least giving Grape a try 😉 As for the packaging side, now I need to update quite some bits, but if @jordimassaguerpla doesn't like the whole idea, or he thinks that it brings more harm than good, then we have two options:

My personal pick: rollback to a version that supports our version of ruby.
Worst case: redirect the API part into mere Rails controllers.

I'm liking grape, and I'm already benefiting of some of its features (check #1384), so I'd like to evaluate all our options before going back to the worst case scenario 😁

vitoravelino · 2017-08-14T12:42:37Z

@mssola I also like Grape and would love to stay with it. I'm just trying to play safe but if you want to go wild... Let's go! Haha...

I've worked in a project with Grape a few years ago (ruby 1.9 era) and it used to do his job. So I guess going with your personal pick wouldn't be a big deal unless we are using a fancy new feature. Based on that I think we are safer than what I thought.

If you approve this PR, we can merge it and I can rewrite the API part in Grape but in another PR.

mssola · 2017-08-14T12:46:17Z

Cool, I'll review later 😉

mssola

Works wonders. Just two things:

Fix the issues I mentioned
Write a more complete commit message. Right now it's pretty raw... So, explain the changes you made and why you made them 😉

mssola · 2017-08-14T14:29:52Z

app/views/namespaces/index.html.slim

+      = render "namespaces/components/panel"
+
+    script#js-namespaces-table-tmpl type="text/x-template"
+      == render "namespaces/components/table"


Typo, should be a single =.

mssola · 2017-08-14T14:29:58Z

app/views/namespaces/index.html.slim

+      = render "namespaces/components/table_row"
+
+    script#js-namespace-visibility-tmpl type="text/x-template"
+      == render "namespaces/components/visibility"


vitoravelino · 2017-08-14T14:36:59Z

app/views/namespaces/components/_table_row.html.slim

@@ -0,0 +1,16 @@
+tr :class="scopeClass"
+  td
+    a{ :href="namespace.links.self.href" }


If :class="scopeClass" works, then :href="..." should also work. Will try without the curly brackets.

vitoravelino · 2017-08-14T14:38:21Z

app/views/namespaces/index.html.slim

+      = render "namespaces/components/panel"
+
+    script#js-namespaces-table-tmpl type="text/x-template"
+      == render "namespaces/components/table"


Maybe the == is not necessary here.

- Created custom elements for namespaces (namespace-panel, new-namespace-form, namespace-table, namespace-table-row, namespace-visibility) - Added sorting columns (similar to SUSE#590) - Fixed pagination (similar to SUSE#585) - Improved UX of changing namespace visibility (see gif below) - Updated slim gem - Updated active_model_serializers gem - Used serializers (check namespace_serializer.rb) for json rendering - Added moment for dates (kind of overkill but a good thing in the long term) - Added lodash and lodash babel plugin (makes partial imports possible) - And other minor changes

vitoravelino · 2017-08-15T22:13:05Z

Since the requested changes were made and travis is passing, I'll merge this by myself to unblock other issues.

vitoravelino force-pushed the namespace-index-vue-refactoring branch 3 times, most recently from 0a8ff43 to a751fe6 Compare August 8, 2017 13:06

vitoravelino commented Aug 8, 2017

View reviewed changes

vitoravelino requested a review from mssola August 8, 2017 13:26

mssola added this to the Release 2.3 milestone Aug 8, 2017

vitoravelino force-pushed the namespace-index-vue-refactoring branch from a751fe6 to 3ba8794 Compare August 8, 2017 15:55

This was referenced Aug 8, 2017

Make namespace table sorting reachable through URL #1373

Closed

Reuse <namespaces-panel> component on Team#show page #1374

Closed

vitoravelino force-pushed the namespace-index-vue-refactoring branch 16 times, most recently from 74eb204 to 4bede10 Compare August 10, 2017 10:35

vitoravelino force-pushed the namespace-index-vue-refactoring branch 2 times, most recently from b061785 to 8626e07 Compare August 10, 2017 12:46

vitoravelino changed the title ~~[WIP] js: Namespaces#index refactoring w/ Vue~~ js: Namespaces#index refactoring w/ Vue Aug 10, 2017

vitoravelino force-pushed the namespace-index-vue-refactoring branch from 8626e07 to 999953a Compare August 14, 2017 11:27

mssola previously approved these changes Aug 14, 2017

View reviewed changes

vitoravelino commented Aug 14, 2017

View reviewed changes

vitoravelino dismissed mssola’s stale review via 6518e29 August 14, 2017 15:13

vitoravelino force-pushed the namespace-index-vue-refactoring branch from 999953a to 6518e29 Compare August 14, 2017 15:13

vitoravelino force-pushed the namespace-index-vue-refactoring branch from 6518e29 to 3e145dc Compare August 15, 2017 21:30

vitoravelino merged commit 83da646 into SUSE:master Aug 15, 2017

vitoravelino mentioned this pull request Aug 17, 2017

Make namespace table pagination reachable through URL #1390

Closed

vitoravelino added the review later label Aug 18, 2017

vitoravelino removed the review later label Aug 31, 2017

vitoravelino deleted the namespace-index-vue-refactoring branch January 3, 2018 10:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

js: Namespaces#index refactoring w/ Vue #1371

js: Namespaces#index refactoring w/ Vue #1371

vitoravelino commented Aug 8, 2017 •

edited by mssola

Loading

vitoravelino Aug 8, 2017

vitoravelino Aug 8, 2017

vitoravelino Aug 8, 2017

vitoravelino commented Aug 10, 2017

mssola commented Aug 14, 2017

vitoravelino commented Aug 14, 2017

vitoravelino commented Aug 14, 2017

mssola commented Aug 14, 2017

vitoravelino commented Aug 14, 2017

mssola commented Aug 14, 2017

mssola left a comment

mssola Aug 14, 2017

mssola Aug 14, 2017

vitoravelino Aug 14, 2017 •

edited

Loading

vitoravelino Aug 14, 2017

vitoravelino commented Aug 15, 2017

js: Namespaces#index refactoring w/ Vue #1371

js: Namespaces#index refactoring w/ Vue #1371

Conversation

vitoravelino commented Aug 8, 2017 • edited by mssola Loading

Missing

vitoravelino Aug 8, 2017

Choose a reason for hiding this comment

vitoravelino Aug 8, 2017

Choose a reason for hiding this comment

vitoravelino Aug 8, 2017

Choose a reason for hiding this comment

vitoravelino commented Aug 10, 2017

mssola commented Aug 14, 2017

vitoravelino commented Aug 14, 2017

vitoravelino commented Aug 14, 2017

mssola commented Aug 14, 2017

vitoravelino commented Aug 14, 2017

mssola commented Aug 14, 2017

mssola left a comment

Choose a reason for hiding this comment

mssola Aug 14, 2017

Choose a reason for hiding this comment

mssola Aug 14, 2017

Choose a reason for hiding this comment

vitoravelino Aug 14, 2017 • edited Loading

Choose a reason for hiding this comment

vitoravelino Aug 14, 2017

Choose a reason for hiding this comment

vitoravelino commented Aug 15, 2017

vitoravelino commented Aug 8, 2017 •

edited by mssola

Loading

vitoravelino Aug 14, 2017 •

edited

Loading