Skip to content
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

Table: Datepicker, AutoComplete, Select Cell Editor #2387

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

ElishaSamPeterPrabhu
Copy link
Collaborator

@ElishaSamPeterPrabhu ElishaSamPeterPrabhu commented Apr 1, 2024

Description

  • Updated dropdown, autocomplete to accept badge and links args and datepicker of cell to handle out of focus

References

#2310 , #2311 , #2313

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

https://deploy-preview-2387--moduswebcomponents.netlify.app/

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@ElishaSamPeterPrabhu ElishaSamPeterPrabhu changed the title Update focus of datepicker Table: Datepicker Cell Editor Apr 1, 2024
Copy link

netlify bot commented Apr 1, 2024

Deploy Preview for moduswebcomponents ready!

Name Link
🔨 Latest commit ec59779
🔍 Latest deploy log https://app.netlify.com/sites/moduswebcomponents/deploys/665d5019d4126300080654c9
😎 Deploy Preview https://deploy-preview-2387--moduswebcomponents.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 24 (🔴 down 1 from production)
Accessibility: 98 (no change from production)
Best Practices: 92 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@coliff coliff requested a review from cjwinsor April 2, 2024 09:36
@ElishaSamPeterPrabhu ElishaSamPeterPrabhu changed the title Table: Datepicker Cell Editor Table: Datepicker,AutoComplete,Select Cell Editor Apr 4, 2024
@cjwinsor
Copy link
Contributor

cjwinsor commented Apr 25, 2024

@ElishaSamPeterPrabhu Some observations:
Storybook:

  • Autocomplete should have all First Names from Names array
  • Autocomplete for the last row will have its items require the user to scroll down in the container to see
  • CreatedAt should be of type Date so we can show that too
  • Email column's Autocomplete options should contain all emails

Component:

  • Autocomplete should set the value
  • Autocomplete should show items on focus
  • Autocomplete, you can't use arrows or tab to navigate the items in the list, table handling takes over. (arrows work for number)
  • Autocomplete increases the row height
  • Date should set value
  • Date column and editor formatting should match
  • Date doesn't lose focus when you click away
  • Date Calendar is inside the overflow container, any way to pop it out? Last item you can look at

@coliff coliff changed the title Table: Datepicker,AutoComplete,Select Cell Editor Table: Datepicker, AutoComplete, Select Cell Editor Apr 30, 2024
@cjwinsor
Copy link
Contributor

cjwinsor commented Jun 3, 2024

@ElishaSamPeterPrabhu I went over these changes again and this is what I think is needed before we can merge:

  • Remove the overflow: auto from .table-container in modus-table.scss. Instead modus-table.tsx should set overflow: auto only when a maxHeight is provided. This way, if no maxHeight is defined then the datepicker and autocomplete won't be hidden inside the overflow container. It doesn't fix for when maxHeight is defined, but at gives a workaround until we can address this.
  • Switch from using "dropdown" for the Select component. Use "select" instead. Update all code to match. This is to avoid confusion with our drop down component we have already which is different.
  • Don't reuse ModusTableCellDropdownEditorArgs for Autocomplete, create a new type for it.
  • ModusTableCellDropdownEditorArgs should have optionsDisplayProp as well, in addition to options.
  • The ModusTableCellAutocompleteEditorArgs which you'll have created above, should have the following additional properties (beyond just options): noResultsFoundText, noResultsFoundSubtext, showNoResultsFoundMessage, showOptionsOnFocus.
  • ModusTableCellAutocompleteEditorArgs.options should be of type ModusAutocompleteOption
  • For autocomplete, it seems if i click on the cell in the whitepace, then click on another cell, it loses focus correctly, but if i click on the text then another cell it doesn't loose focus. Might be related to the example being a link, I didn't check with a non link autocomplete column
  • I think we need to make sure ModusTableCellEditorType in storybook and code shows and supports all the different cell editor types, it currently only shows type ModusTableCellEditorType = typeof CELL_EDIT_TYPE_DROPDOWN;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants