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.

Sign up for GitHub

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

Jump to bottom

Audit and Fix types.md #102

Closed
wants to merge 10 commits into from
Closed

Audit and Fix types.md #102

wants to merge 10 commits into from

Conversation

lmpham1
Copy link

@lmpham1 lmpham1 commented Nov 18, 2021 •
edited
Loading

Audit and Fix types.md. Closes #90

Changes made

  • Fixed markdown errors
  • Change the long HTML tables (the ones representing memory) to SVG for scalability (all except one, see this comment below)
  • Added an alt value to the image
  • Switched the image to fetch from the static/img/ folder instead
  • Added Frontmatter
  • Changed the tables under the Value Ranges section from HTML to Markdown

@lmpham1
changed most of the table to svg, except double representation
d6986d5 
switch to using imgs from static/img folder instead

changed the tables in Value Ranges section from HTML to Markdown tables

added frontmatter

added types.png to static folder
@lmpham1
Copy link
Author

lmpham1 commented Nov 18, 2021

So for most tables that represent memory storage, I converted them to svg to make them responsive. I also split the ones that are too long into different rows so that they don't look too small when scaled down, something like this:
image

However, there is one table left that I don't know what the best approach for it is. It is the table representing double, which is 8-byte long:
image

For other 8-byte long tables, I just converted them to SVG and split them into two rows of 4-byte length. However, I can't think of a way to split this table nicely as the cells in the third row (the s, exponent, and significand row) are not equal in length. If I leave it as one row and try to convert that to SVG to fit the viewport, it will become too small to see.

humphd
humphd requested changes Nov 18, 2021
View reviewed changes
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

One thing I want to figure out in a follow-up is how to add a class in dark-mode to alter the CSS applied to the SVG tables, since they need a different background colour there.

If we can make this SVG/Table approach work, it's really cool.

docs/B-Computations/types.md Outdated

A **`long long int`** \(or simply, a `long long`\) contains at least 64 bits:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 980 260">
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to not have the table row wrap onto another line: the entire thing needs to fit in a single row across, and scale it to fit within the parent container.

Copy link
Author

Choose a reason for hiding this comment

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

for the 8-byte length tables, I feel like it's a bit too small fitting everything in a row

image

Even smaller in mobile:
image

docs/B-Computations/types.md Outdated
Comment on lines 46 to 40

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 800 165">
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a really clever approach to use SVG for this.

irenejoeunpark
irenejoeunpark reviewed Nov 18, 2021
View reviewed changes
Copy link

@irenejoeunpark irenejoeunpark left a comment

Choose a reason for hiding this comment

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

Great work!
The alignment of the documentation is different for the top part and the rest.

docs/B-Computations/types.md Outdated Show resolved Hide resolved
@lmpham1
changed most of the table to svg, except double representation
0791961 
switch to using imgs from static/img folder instead

changed the tables in Value Ranges section from HTML to Markdown tables

added frontmatter

added types.png to static folder

Made image centered

Fixed trailing spaces after sentence breaks

Made all memory tables one row height

Adjusted alignment of the small tables (the ones that don't represent memory)
@lmpham1
fixed more trailing and leading spaces
5daf553
@tpmai22 tpmai22 mentioned this pull request Nov 22, 2021
Merged
humphd
humphd requested changes Nov 25, 2021
View reviewed changes
docs/B-Computations/types.md Outdated Show resolved Hide resolved
docs/B-Computations/types.md Show resolved Hide resolved
@CameronGray1210
Copy link
Collaborator

@lmpham1: Is there an update to this?

@humphd
Copy link
Contributor

humphd commented Dec 2, 2021

@lmpham1 can you please update your branch and run yarn and yarn prettier so you can fix the formatting issues here?

@humphd humphd mentioned this pull request Dec 3, 2021
Closed
@Kevan-Y
Copy link
Contributor

Kevan-Y commented Dec 3, 2021

@lmpham1 Hi, there is a recent change where we remove slug (#142). Could you kindly remove slug in all you changed files, thank you

@humphd humphd mentioned this pull request Dec 16, 2021
Merged
@humphd
Copy link
Contributor

humphd commented Dec 16, 2021

Closing in favour of https://github.com/Seneca-ICTOER/IPC144/pull/154

@humphd humphd closed this Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tpmai22 tpmai22 tpmai22 left review comments

@irenejoeunpark irenejoeunpark irenejoeunpark left review comments

@DerekJxy DerekJxy DerekJxy left review comments

@humphd humphd humphd requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Audit and fix types.md
7 participants
@lmpham1 @CameronGray1210 @humphd @Kevan-Y @tpmai22 @irenejoeunpark @DerekJxy