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

feat(vwc-calendar): calendar 1st draft #564

Merged
merged 18 commits into from
Jan 17, 2021
Merged

feat(vwc-calendar): calendar 1st draft #564

merged 18 commits into from
Jan 17, 2021

Conversation

yinonov
Copy link
Contributor

@yinonov yinonov commented Jan 10, 2021

  • Alpha version of calendar. should be exposed to promote and get feedback on how to move forward with its API (currently no API featured)

  • no existing tests

@github-actions
Copy link

🚀

Latest successful build of the PR deployed here.

🚀

@yinonov yinonov marked this pull request as draft January 11, 2021 06:55
@github-actions
Copy link

github-actions bot commented Jan 11, 2021

File Coverage
All files 80%
common/context/src/vvd-context.ts 96%
common/fonts/src/vvd-fonts.ts 83%
common/foundation/src/form-association/associate-with-form.ts 90%
common/foundation/src/form-association/common.ts 90%
common/foundation/src/form-association/submit-on-enter-key.ts 80%
common/scheme/src/os-sync.utils.ts 58%
common/scheme/src/vvd-scheme-style-tag-handler.ts 79%
common/scheme/src/vvd-scheme.ts 82%
components/audio/src/vwc-audio.ts 60%
components/button/src/vwc-button.ts 79%
components/carousel/src/vwc-carousel.ts 71%
components/drawer/src/vwc-drawer.ts 42%
components/fab/src/vwc-fab.ts 63%
components/file-picker/src/vwc-file-picker.ts 67%
components/icon-button-toggle/src/vwc-icon-button-toggle.ts 67%
components/icon-button/src/vwc-icon-button.ts 88%
components/icon/src/vwc-icon.js 92%
components/list/src/vwc-list-expansion-panel.ts 73%
components/list/src/vwc-list-item.ts 81%
components/media-controller/src/vwc-media-controller.ts 84%
components/select/src/vwc-select.ts 90%
components/slider/src/vwc-slider.ts 88%
components/textarea/src/vwc-textarea.ts 81%
components/textfield/src/vwc-textfield.ts 90%
components/theme-switch/src/vwc-theme-switch.ts 88%

Minimum allowed coverage is 75%

Generated by 🐒 cobertura-action against 16393b7

@yinonov yinonov marked this pull request as ready for review January 17, 2021 07:37
<ol class="calendar">
${this.renderTimeCells()}
<!-- TODO: should be presented as a custom element. then could look for siblings and indent by js -->
<section
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can <section> be a child of <ol>?

Copy link
Contributor Author

@yinonov yinonov Jan 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's correct, but later we'd might use a custom element for that section, that's a pickle that is also issued in mwc-list. any suggestions?

Copy link
Contributor

@tveinfeld tveinfeld Jan 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ones that will extend li?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, no extending native elements, not supported on Safari, etc.
But could an li element hold this content? Yes it can, so just anything we'd like to push there may be put into the li, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will <vwc-event> with <li> in its shadow do?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, anything will do - the HTML is not restrictive, but if we want to stick to it, as per @tveinfeld comment - ol direct children may only be li (or template or script, i think too, but not relevant).
So if any - i'd say li element and within it vwc-event and not otherwise.
...and of course, given that it's our component and given that you actually not really using a list features here and even semantically it is not strictly it - just drop the li and use anything else - IMHO, that would be the simplest

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll follow the material pattern which is an li within a custom element wrapper. in the next calendar PR. it is a 1st draft

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accepted

@sonarcloud
Copy link

sonarcloud bot commented Jan 17, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@gullerya gullerya merged commit b0c5162 into master Jan 17, 2021
@gullerya gullerya deleted the vwc-calendar branch January 17, 2021 14:21
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

3 participants