You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by robertkowalski <gi...@git.apache.org> on 2016/06/21 09:52:36 UTC
[GitHub] couchdb-fauxton pull request #732: Breadcrumbs React port: cleanup
GitHub user robertkowalski opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/732
Breadcrumbs React port: cleanup
Clean up un-needed, not used link properties. They would link in
a breadcrumb system which supports links.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertkowalski/couchdb-fauxton cleanup-read-refactor
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/732.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #732
----
commit 5b96fbf71ed8ca01ef3eff12e726feadf8f5402a
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2016-06-21T09:52:18Z
Breadcrumbs React port: cleanup
Clean up un-needed, not used link properties. They would link in
a breadcrumb system which supports links.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #732: Breadcrumbs React port: cleanup
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/732
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton issue #732: Breadcrumbs React port: cleanup
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the issue:
https://github.com/apache/couchdb-fauxton/pull/732
+1 looks great. Nice work.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #732: Breadcrumbs React port: cleanup
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/732#discussion_r68575338
--- Diff: app/addons/components/header-breadcrumbs.jsx ---
@@ -0,0 +1,71 @@
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+import React from 'react';
+import ReactDOM from 'react-dom';
+
+export const Breadcrumbs = ({crumbs}) => {
+
+ const childs = getChildren(crumbs);
+
+ return (
+ <div className="faux-header__breadcrumbs">
+ {childs}
+ </div>
+ );
+
+};
+
+Breadcrumbs.propTypes = {
+ crumbs: React.PropTypes.array.isRequired
+};
+
+
+const CrumbElement = ({children}) => {
+ return <div className="faux-header__breadcrumbs-element">{children}</div>;
+};
+
+const Divider = () => {
+ const style = {width: '13px', paddingTop: '22px', fontSize: '20px'};
--- End diff --
Can we not put these style rules in `faux-header__breadcrumbs-divider`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton issue #732: Breadcrumbs React port: cleanup
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the issue:
https://github.com/apache/couchdb-fauxton/pull/732
Code looks good. Two issues though. When I'm editing a doc, there is no way to go back to all docs other than pressing cancel
Also if I press clone doc, and then in the modal I type a new id and then press enter the page navigates to the wrong page.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #732: Breadcrumbs React port: cleanup
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/732#discussion_r68575205
--- Diff: app/addons/components/header-breadcrumbs.jsx ---
@@ -0,0 +1,71 @@
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+import React from 'react';
+import ReactDOM from 'react-dom';
+
+export const Breadcrumbs = ({crumbs}) => {
+
+ const childs = getChildren(crumbs);
--- End diff --
You can just call getChildren in the jsx section. No point in adding this variable.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #732: Breadcrumbs React port: cleanup
Posted by BenJanecke <gi...@git.apache.org>.
Github user BenJanecke commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/732#discussion_r68552967
--- Diff: app/addons/components/assets/less/header-breadcrumbs.less ---
@@ -0,0 +1,35 @@
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+.faux-header__breadcrumbs {
--- End diff --
Great to see some BEM! \U0001f44d Nice composable little bits, great job @robertkowalski
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton issue #732: Breadcrumbs React port: cleanup
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the issue:
https://github.com/apache/couchdb-fauxton/pull/732
@garrensmith TIL you pure functional components need a wrapper div to render in tests: ab37827
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #732: Breadcrumbs React port: cleanup
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/732#discussion_r68575487
--- Diff: app/addons/components/stores.js ---
@@ -117,7 +117,35 @@ Stores.DeleteDbModalStore = FauxtonAPI.Store.extend({
}
});
+Stores.BreadCrumbStore = FauxtonAPI.Store.extend({
--- End diff --
I think you can remove this and the actions
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---