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/06/26 22:22:34 UTC

[GitHub] [incubator-superset] etr2460 opened a new pull request #10179: fit: row count container alignment

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


   ### SUMMARY
   It looks like this change wasn't wasn't tested for both sides of this feature flag `LIST_VIEWS_NEW_UI` or the config `ENABLE_REACT_CRUD_VIEWS` (uncertain how these actually interact with each other)
   
   hopefully we can reach feature parity soon so that we can all migrate to having it True, reducing the complexity of working around this
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   Before:
   <img width="1920" alt="Screen Shot 2020-06-26 at 3 15 29 PM" src="https://user-images.githubusercontent.com/7409244/85905297-e5303600-b7bf-11ea-99c1-de2748451daf.png">
   
   After:
   <img width="1920" alt="Screen Shot 2020-06-26 at 3 09 53 PM" src="https://user-images.githubusercontent.com/7409244/85905165-85d22600-b7bf-11ea-8c9e-9a19c5e98adc.png">
   
   ### TEST PLAN
   Ensure the row count container is aligned properly
   
   Even after following the instructions from your PR (enabling the feature flag) @nytai, I wasn't able to get the new UI to show up. Could you direct me how to test that this works with the new styling?
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [x] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   to: @nytai @graceguo-supercat @ktmud 


----------------------------------------------------------------
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] nytai commented on pull request #10179: fix: row count container alignment

Posted by GitBox <gi...@apache.org>.
nytai commented on pull request #10179:
URL: https://github.com/apache/incubator-superset/pull/10179#issuecomment-650432883


   @etr2460 The `LIST_VIEWS_NEW_UI` refers to some changes in the listview filters, it's unrelated to this change. Sorry for the confusing name, it should be USE_NEW_FILTER_UI or something similar. Both charts, dashboards, and datasets have the new UI under `ENABLE_REACT_CRUD_VIEWS`, which makes the tables full width, hence the absolute positioning on the row count. Looks like I missed checking the welcome page which is also using the ListView component. 
   
   The way to fix this is to either make the welcome page full-width or go back to the `float: right` styling that was in place before (which should mostly be the same) 


----------------------------------------------------------------
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] etr2460 merged pull request #10179: fix: row count container alignment

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


   


----------------------------------------------------------------
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] nytai commented on a change in pull request #10179: fix: row count container alignment

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



##########
File path: superset-frontend/src/components/ListView/ListViewStyles.less
##########
@@ -134,6 +134,10 @@
     }
 
     .row-count-container {
+      float: right;

Review comment:
       let's just change this to 
   ```css
   float: right;
   margin-right: 20px;
   ```
   
   which should be enough to make this look OK on both the `/superset/welcome` and the `/tablemodelview/list/` pages with `ENABLE_REACT_CRUD_VIEWS` flag enabled. The `LIST_VIEWS_NEW_UI` flag is only related to the filters ui, you can update it to `LIST_VIEWS_NEW_FILTER_UI` to be more semantically correct if you're up for it. 




----------------------------------------------------------------
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