-
Notifications
You must be signed in to change notification settings - Fork 54
feat(ssd1306): added 128X64 OLED #13
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
src/ssd1306-element-element.ts
Outdated
firstUpdated() { | ||
/// this.putGivenImageData(); | ||
// Getting null for canvas, there's an issue here | ||
this.testImageData(); |
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.
@urish maybe you'll have an idea, at this point I expected to have the rendered DOM with the canvas element, but querying it will resolve as null so I can't test the image data, (+ the size of the foreignObject and hosted canvas isn't right as well)
src/ssd1306-element-element.ts
Outdated
@property() imageData: ImageData = new ImageData(128, 64); | ||
|
||
putGivenImageData() { | ||
const canvas: HTMLCanvasElement | null = document.querySelector('canvas'); |
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 lit-element
, the components are enclosed in Shadow DOM, so there is an API for accessing them, called shadowRoot
. You can see an example here: https://lit-element.polymer-project.org/guide/lifecycle#examples
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.
thanks :) it works
src/ssd1306-element-element.ts
Outdated
export class Ssd1306ElementElement extends LitElement { | ||
@property() dimensions: NumTuple = [141, 108]; | ||
@property() bitmap: any; // size should be 128 x 64 | ||
@property() dimensions: Dimensions = [141, 108]; |
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.
It'd be better to have separate width
/ height
properties, this way they can also be set via plain HTML when creating an instance of you component (also, it's pretty standard in HTML, e.g. the <img>
, <svg>
and <canvas>
tags all follow this convention)
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.
done
src/ssd1306-element-element.ts
Outdated
<!-- 128 x 64 screen --> | ||
<path fill="#1A1A1A" d="M21 26h100v54H21z" /> | ||
<foreignObject transform="translate(4 6)" x="20" y="20" width="128" height="64"> |
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.
Is there a reason we need both translate
and x
/ y
?
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.
nope I fixed :)
@urish I had some proportions issues with the original size (not 100% sure why it happened, I used the original metrics..) so I stayed loyal to the oled screen size and did some small changed to the svg, LMK what do you think? I changed the width/hight accessors to be fixed to a const size instead of being a user input (cause I don't draw the svg dynamically at this point, an interesting challenge though...) |
src/ssd1306-128x64-element.ts
Outdated
text-anchor="middle" | ||
font-size="5" | ||
font-weight="300" | ||
font-family="MarkerFelt-Wide, Marker Felt" |
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.
These fonts are not available on all computers - let's add a fallback from one of the web-safe fonts (https://www.w3schools.com/cssref/css_websafe_fonts.asp)
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.
done
src/ssd1306-128x64-element.ts
Outdated
} | ||
|
||
putGivenImageData() { | ||
const canvas: HTMLCanvasElement | null | undefined = this.shadowRoot?.querySelector('canvas'); |
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.
Could possibly be written as
const canvas = this.shadowRoot?.querySelector<HTMLCanvasElement>('canvas');
1e34060
to
64ac5bf
Compare
src/ssd1306-element.stories.ts
Outdated
() => | ||
html` | ||
<wokwi-ssd1306-element | ||
updateImage="${boolean('updateImage', false)}" |
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 no longer have updateImage
:-)
so I guess we can remove this 2nd story altogether
src/ssd1306-element.stories.ts
Outdated
import { html } from 'lit-html'; | ||
import './ssd1306-element'; | ||
|
||
storiesOf('Ssd1306 Element', module) |
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 can remove "Element" from the name (just to stay consistent with the other stories), also call it SSD1306 (like we used uppercase in LED and LCD1602)
No description provided.