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

Added truth tables for demux #432

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

Dishebh
Copy link
Contributor

@Dishebh Dishebh commented Jun 9, 2020

Changes done:

  • Added truth tables for demux

Screenshots

Preview Link(s): https://deploy-preview-432--tender-ardinghelli-ebb79f.netlify.app/docs/MSI/demux.html

✅️ By submitting this PR, I have verified the following

  • Checked to see if a similar PR has already been opened 🤔️
  • Reviewed the contributing guidelines 🔍️
  • Sample preview link added (add the link(s) for all the pages changed/updated from the checks tab after checks complete)
  • Tried Squashing the commits into one

Copy link
Member

@Shivansh2407 Shivansh2407 left a comment

Choose a reason for hiding this comment

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

Please remove Block Diagram from TOC @Dishebh 👍. Also please let me know your views on making the table as it was implemented in the mix page. Is there any specific reason of using HTML table tags here?

@Dishebh
Copy link
Contributor Author

Dishebh commented Jun 9, 2020

@Shivansh2407 the tables had a complex structure here (if you compare it with the other tables) as it had nested inputs and outputs, similar to jk_race_around_condition table. And so, I used the table tags and implemented it this way.

@Dishebh
Copy link
Contributor Author

Dishebh commented Jun 11, 2020

please review @Shivansh2407 @YashKumarVerma :)

Copy link
Member

@Shivansh2407 Shivansh2407 left a comment

Choose a reason for hiding this comment

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

Please Shadow Effect to each and every side of the Table . Take reference from the Race around Condition Page. Currently the shadow is available in the bottom of the tables and side borders are there only for the second row of each table. You need to remove that left border from first column of second row and add shadow effects to each side to complete the table.
Please make the suggested changes @Dishebh 👍

@Shivansh2407
Copy link
Member

Have a look at the image below :
Current Table :
image
Race Around Page Table :
image

@Dishebh
Copy link
Contributor Author

Dishebh commented Jun 11, 2020

@Shivansh2407 made the box-shadow same as jk_race_around_condition truth table :)

@Shivansh2407
Copy link
Member

@Dishebh Please look at the 1:8 and 1:16 table as these are horizontally long and in desktop view no horizontal scrolling is enabled. Try to implement the horizontal scroll in these two tables so that the user can view the rest of the table.

@Dishebh
Copy link
Contributor Author

Dishebh commented Jun 12, 2020

@Shivansh2407 I hope it fixes the scrolling issue now

@Shivansh2407
Copy link
Member

@Shivansh2407 I hope it fixes the scrolling issue now

The scrolling issue is fixed but the problem now is a white space that is added to each and every table . Just fix this and we will be ready to merge this @Dishebh 👍

Have a look at the image below :
image

@Dishebh
Copy link
Contributor Author

Dishebh commented Jun 12, 2020

@Shivansh2407 removed extra space below tables

@Shivansh2407
Copy link
Member

Shivansh2407 commented Jun 12, 2020

@Dishebh the thing is I want you to make margin bottom available for the tables with scrolling as it was earlier. For tables without scroll bar remove that thing. Also in the recent commit, you have removed the spacing between the Truth table and the circuit. Please fix that. If you have any questions, please let us know.

Copy link
Member

@YashKumarVerma YashKumarVerma left a comment

Choose a reason for hiding this comment

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

Check hound errors

@Dishebh
Copy link
Contributor Author

Dishebh commented Jun 12, 2020

@Shivansh2407 I have set the overflow property of table to auto, and the margin-bottom to 0. This ensures that:

  • The scroll-bar gets automatically added to the table, depending upon the screen resolution and size.
  • The extra space visible at the bottom of the table gets vanished and resolves the issue:

The scrolling issue is fixed but the problem now is a white space that is added to each and every table.

So it would be hard to determine which table has scroll-bar and which has not, as it will never be a fixed bool. And if I add a margin-bottom, it will again give rise to the previous issue.

Copy link
Member

@YashKumarVerma YashKumarVerma left a comment

Choose a reason for hiding this comment

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

Check hound issues.

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.

3 participants