- 
                Notifications
    
You must be signed in to change notification settings  - Fork 1
 
LPD-24670 | Inform about Data Sets without visualization mode when selecting Data Set #5122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| 
           CI is automatically triggering the following test suites: 
  | 
    
| 
           Test suite sf has been triggered on http://test-1-38  | 
    
          ✔️ ci:test:sf - 1 out of 1 jobs passed in 4 minutesRan com.liferay.source.formatter at released version 1.0.1543. Click here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPD-24670 1 Successful Jobs:For more details click here. | 
    
| 
           Jenkins Build:test-portal-source-format#6642 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-frontend#5122 Testray Routine:EE Pull Request Testray Build:[master] ci:test:sf - chestofwonders > liferay-frontend - PR#5122 - 2025-10-10[03:20:25] Testray Build ID: 336937551Testray Importer:test-portal-source-format#6642  | 
    
| 
           @chestofwonders pls check comment on old, draft PR Also, pls remove "WIP" from commit message :)  | 
    
f4b4fa6    to
    db9686f      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chestofwonders pls see inline comments
        
          
                ...les/apps/portal-language/portal-language-lang/src/main/resources/content/Language.properties
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...a-set-admin-web/src/main/resources/META-INF/resources/item/selector/FDSAdminItemSelector.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...a-set-admin-web/src/main/resources/META-INF/resources/item/selector/FDSAdminItemSelector.tsx
          
            Show resolved
            Hide resolved
        
              
          
                ...a-set-admin-web/src/main/resources/META-INF/resources/item/selector/FDSAdminItemSelector.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...end-data-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/list/List.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...end-data-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/list/List.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
      6f9d530    to
    501b5fa      
    Compare
  
    c549eca    to
    1a6c46b      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nit
        
          
                ...end-data-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/list/List.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chestofwonders please see comments inline
        
          
                ...a-set-admin-web/src/main/resources/META-INF/resources/item/selector/FDSAdminItemSelector.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...a-set-admin-web/src/main/resources/META-INF/resources/item/selector/FDSAdminItemSelector.tsx
          
            Show resolved
            Hide resolved
        
              
          
                ...end-data-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/list/List.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...end-data-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/list/List.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...end-data-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/list/List.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...end-data-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/list/List.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...end-data-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/list/List.tsx
              
                Outdated
          
            Show resolved
            Hide resolved
        
      1a6c46b    to
    2f72fb2      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ;) pls see inline comments. Just minor nit on test description
| sticker: 'sticker', | ||
| symbol: 'symbol', | ||
| title: 'label', | ||
| tooltip: 'tooltip', | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that List.tsx handles everything via schema we don't need hidden conventions in order to pass the information we need ✨
| <ClayIcon symbol={item[symbol]} /> | ||
| </ClaySticker> | ||
| </span> | ||
| </ClayTooltipProvider> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are rendering the tooltip, if present, associated woth the symbol. This is as opinionated as doing it in other places. Decision follows designs, it's fine. In the future, placement of tooltip can be changed/tweaked if needed 👍
| <ClayList.ItemField> | ||
| {(itemsActions || item.actionDropdownItems) && ( | ||
| {(itemsActions || item.actionDropdownItems) && ( | ||
| <ClayList.ItemField> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch avoiding an empty ItemField component here when there are no actions
| symbol: 'exclamation-circle', | ||
| tooltip: Liferay.Language.get( | ||
| 'no-visualization-modes-have-been-defined' | ||
| ), | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 With this change, now the concept of "valid" is meaningless for FDS, which limits itself to render what's told to. Parent component here is in charge of defining the intent of the item depending on custom conditions.
This is the right pattern!
        
          
                ...t/playwright/tests/frontend-data-set-admin-web/main/tests/data-set-fragment/dataSets.spec.ts
              
                Outdated
          
            Show resolved
            Hide resolved
        
      2f72fb2    to
    29c5f25      
    Compare
  
    | 
           ci:forward  | 
    
| 
           Test suite relevant has been triggered on http://test-1-32  | 
    
| 
           Test suite sf has been triggered on http://test-1-32  | 
    
| 
           CI is automatically triggering the following test suites: 
 The pull request will automatically be forwarded to the user  
  | 
    
          ✔️ ci:test:sf - 1 out of 1 jobs passed in 4 minutesRan com.liferay.source.formatter at released version 1.0.1548. Click here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPD-24670 1 Successful Jobs:For more details click here. | 
    
| 
           Jenkins Build:test-portal-source-format#11181 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-frontend#5122 Testray Routine:EE Pull Request Testray Build:[master] ci:test:sf - chestofwonders > liferay-frontend - PR#5122 - 2025-10-29[12:21:53] Testray Build ID: 353962452Testray Importer:test-portal-source-format#11181  | 
    
          ✔️ ci:test:stable - 11 out of 11 jobs passed❌ ci:test:relevant - 14 out of 17 jobs passed in 58 minutesClick here for more details.Base Branch:Branch Name: master Upstream Comparison:Branch GIT ID: cb2950353b3968cfa6c10a0bd3dfebb8e8beafc3 ci:test:stable - 11 out of 11 jobs PASSED11 Successful Jobs:ci:test:relevant - 14 out of 17 jobs PASSED3 Failed Jobs:
 14 Successful Jobs:For more details click here.Failures unique to this pull:For upstream results, click here.Test bundle downloads: | 
    
| 
           Jenkins Build:test-portal-acceptance-pullrequest(master)#8583 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-frontend#5122 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - chestofwonders > liferay-frontend - PR#5122 - 2025-10-29[12:21:53] Testray Build ID: 354045632Testray Importer:test-portal-acceptance-pullrequest(master)#8583  | 
    
| 
           ci:forward:force  | 
    
| 
           CI is automatically triggering the following test suites: 
 The pull request will automatically be forwarded to the user  
  | 
    
| 
           Skipping previously completed test suites: 
  | 
    
| 
           Test suite default has been triggered on http://test-1-32  | 
    
| 
           All required test suite(s) completed.  | 
    
| 
           Pull request has been successfully forwarded to  brianchandotcom#167224  | 
    
Issue: https://liferay.atlassian.net/browse/LPD-24670
See #5110 for previous discussion
Inform about Data Sets without visualization mode when selecting Data Set
The goal of this PR is to warn the user when selecting a Data Set from the list to display in a fragment, that there are Data Set that will not be able to be displayed on the page, since no visualization modes have been defined for them.
Also, a tooltip is show when user hovers the cursor over the warning icon