You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/10/01 05:31:54 UTC

[GitHub] [incubator-superset] mistercrunch opened a new pull request #11127: fix: SpatialControl popover won't open

mistercrunch opened a new pull request #11127:
URL: https://github.com/apache/incubator-superset/pull/11127


   Recently a change related to emotion styling not properly working through
   react-bootstrap's popover broke the SpatialControl.
   
   This PR makes SpatialControl use antd's equivalent, and addresses the
   issue as a result: emotion's styling context is preserved through
   this superior popover.
   
   <img width="462" alt="Screen Shot 2020-09-30 at 10 30 54 PM" src="https://user-images.githubusercontent.com/487433/94771694-b2707d80-036c-11eb-91ff-dbd82f25a9e6.png">
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #11127: fix: SpatialControl popover won't open

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #11127:
URL: https://github.com/apache/incubator-superset/pull/11127#discussion_r497998128



##########
File path: superset-frontend/src/components/Popover/index.tsx
##########
@@ -0,0 +1,23 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 { Popover } from 'src/common/components';
+
+// Eventually Popover can be wrapped and customized in this file
+// for now we're just redirecting
+export default Popover;

Review comment:
       Yeah I didn't know if we wanted to do this redirect here. Main goal was to fix `SpatialControl`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on a change in pull request #11127: fix: SpatialControl popover won't open

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11127:
URL: https://github.com/apache/incubator-superset/pull/11127#discussion_r497995601



##########
File path: superset-frontend/src/components/Popover/index.tsx
##########
@@ -0,0 +1,23 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 { Popover } from 'src/common/components';
+
+// Eventually Popover can be wrapped and customized in this file
+// for now we're just redirecting
+export default Popover;

Review comment:
       Depending on how we feel about the appearance of the popover, we may not _need_ to do this, _but_ I like this scaffolding to set us up for a little Emotion styling here. If the day comes that we _do_ tweak the styles, let's just be sure to add it to Storybook! 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] mistercrunch merged pull request #11127: fix: SpatialControl popover won't open

Posted by GitBox <gi...@apache.org>.
mistercrunch merged pull request #11127:
URL: https://github.com/apache/incubator-superset/pull/11127


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-commenter commented on pull request #11127: fix: SpatialControl popover won't open

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #11127:
URL: https://github.com/apache/incubator-superset/pull/11127#issuecomment-701898146


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11127?src=pr&el=h1) Report
   > Merging [#11127](https://codecov.io/gh/apache/incubator-superset/pull/11127?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e7a4265c306096d9fc2be46f23f860e9a5fec4f4?el=desc) will **decrease** coverage by `5.62%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11127/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11127?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11127      +/-   ##
   ==========================================
   - Coverage   65.73%   60.10%   -5.63%     
   ==========================================
     Files         819      385     -434     
     Lines       38865    24323   -14542     
     Branches     3669        0    -3669     
   ==========================================
   - Hits        25546    14619   -10927     
   + Misses      13209     9704    -3505     
   + Partials      110        0     -110     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #cypress | `?` | |
   | #javascript | `?` | |
   | #python | `60.10% <ø> (-1.08%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11127?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11127/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/11127/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.91% <0.00%> (-59.58%)` | :arrow_down: |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11127/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
   | [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/11127/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/11127/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.32%)` | :arrow_down: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/11127/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
   | [superset/databases/commands/update.py](https://codecov.io/gh/apache/incubator-superset/pull/11127/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11127/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `81.38% <0.00%> (-7.98%)` | :arrow_down: |
   | [superset/databases/dao.py](https://codecov.io/gh/apache/incubator-superset/pull/11127/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.11% <0.00%> (-5.89%)` | :arrow_down: |
   | [superset/views/database/validators.py](https://codecov.io/gh/apache/incubator-superset/pull/11127/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvdmFsaWRhdG9ycy5weQ==) | `78.94% <0.00%> (-5.27%)` | :arrow_down: |
   | ... and [450 more](https://codecov.io/gh/apache/incubator-superset/pull/11127/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11127?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11127?src=pr&el=footer). Last update [e7a4265...1ddd7ee](https://codecov.io/gh/apache/incubator-superset/pull/11127?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on a change in pull request #11127: fix: SpatialControl popover won't open

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11127:
URL: https://github.com/apache/incubator-superset/pull/11127#discussion_r497998765



##########
File path: superset-frontend/src/components/Popover/index.tsx
##########
@@ -0,0 +1,23 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 { Popover } from 'src/common/components';
+
+// Eventually Popover can be wrapped and customized in this file
+// for now we're just redirecting
+export default Popover;

Review comment:
       I think it's a good pattern to do this, actually. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org