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

docs: A number of UI changes for Docusaurus #1366

Merged
merged 11 commits into from
Dec 5, 2023
Merged

docs: A number of UI changes for Docusaurus #1366

merged 11 commits into from
Dec 5, 2023
+324 −43

Conversation

mmulji-ic
Copy link
Contributor

@mmulji-ic mmulji-ic commented Oct 17, 2023 •
edited
Loading

Description

Updates the Docusarus theme with the following items:

  • Updates the copyright message
  • Fixes the footer, making the text visible
  • Fixes the footer Cosmos image link
  • Updates the Next/Prev card tags so that they are more easily visible
  • Adds the Hub icon to the theme
  • Adds a light mode

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct docs: prefix in the PR title
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • Confirmed the correct docs: prefix in the PR title
  • Confirmed all author checklist items have been addressed
  • Confirmed that this PR only changes documentation
  • Reviewed content for consistency
  • Reviewed content for spelling and grammar
  • Tested instructions (if applicable)
  • Checked that the documentation website can be built and deployed successfully (run make build-docs)

@mmulji-ic mmulji-ic requested a review from a team as a code owner October 17, 2023 10:11
@p-offtermatt
Copy link
Contributor

Thanks, checked it out locally and afaict everything looks good

@mmulji-ic mmulji-ic self-assigned this Oct 18, 2023
@mmulji-ic mmulji-ic added scope: docs Improvements or additions to documentation S: ImprovingThings Improving things: Customer requests, performance improvements, reliability and usability labels Oct 18, 2023
MSalopek
MSalopek requested changes Oct 31, 2023
View reviewed changes
Copy link
Contributor

@MSalopek MSalopek left a comment

Choose a reason for hiding this comment

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

Not sure the deploy works. Did you make a test deploy?

docs/docusaurus.config.js Outdated Show resolved Hide resolved
docs/docusaurus.config.js Outdated Show resolved Hide resolved
docs/src/css/custom.css Outdated
@@ -355,12 +359,13 @@ html {

/* RELATED ARTICLES */
&[data-theme="dark"] .pagination-nav > a {
@apply bg-fg border-stone-800;
@apply bg-fg;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't know why you added border?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was aligning with the hub docs site.

Copy link
Contributor Author

@mmulji-ic mmulji-ic Dec 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

actually removed, but reverting anyway for the time being

docs/src/css/custom.css Outdated
}
blockquote {
@apply my-7;
}
a {
@apply underline;
/* color: var(--ifm-color-primary ); */
@apply no-underline hover:underline;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the color choice is poor, it will be impossible to figure out which part of the text is a link.
That's why it was underlined in the first place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverting

@p-offtermatt
Copy link
Contributor

p-offtermatt commented Nov 1, 2023 •
edited
Loading

Checked back in on this and noticed that dark theme works pretty poorly with some things here,
particularly links and highlighting text.
image
highlighted text is not legible
image
links indistinguishable

compare these in light mode:
image
highlighted text is readable
image
Link clearly visible

@mmulji-ic
Copy link
Contributor Author

highlighted text is readable

Locally yes, also @p-offtermatt did I believe.

@mmulji-ic
Copy link
Contributor Author

Checked back in on this and noticed that dark theme works pretty poorly with some things here, particularly links and highlighting text. image highlighted text is not legible image links indistinguishable

compare these in light mode: image highlighted text is readable image Link clearly visible

Light mode is the way to go.

@p-offtermatt
Copy link
Contributor

Checked back in on this and noticed that dark theme works pretty poorly with some things here, particularly links and highlighting text. image highlighted text is not legible image links indistinguishable
compare these in light mode: image highlighted text is readable image Link clearly visible

Light mode is the way to go.

Could we still change it so that it works properly in dark mode? We should be able to take the settings that work from the sdk and just change links to be more distinguishable

@mmulji-ic
Copy link
Contributor Author

Could we still change it so that it works properly in dark mode? We should be able to take the settings that work from the sdk and just change links to be more distinguishable

The changes from SDK version aren't that significant.

@MSalopek MSalopek force-pushed the mmulji-ic/update-ics-docs-theme branch from 661d055 to 7b7a5f8 Compare December 4, 2023 17:59
@MSalopek
Copy link
Contributor

MSalopek commented Dec 4, 2023

Checked back in on this and noticed that dark theme works pretty poorly with some things here, particularly links and highlighting text.

Resolved a69ff73

@MSalopek MSalopek self-requested a review December 4, 2023 19:49
MSalopek
MSalopek approved these changes Dec 4, 2023
View reviewed changes
Copy link
Contributor

@MSalopek MSalopek left a comment

Choose a reason for hiding this comment

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

Added some small changes and fixed deps.

@github-actions github-actions bot added C:Docs Assigned automatically by the PR labeler C:ADR Assigned automatically by the PR labeler labels Dec 5, 2023
@MSalopek MSalopek added this pull request to the merge queue Dec 5, 2023
Merged via the queue into main with commit 583d178 Dec 5, 2023
14 checks passed
@MSalopek MSalopek deleted the mmulji-ic/update-ics-docs-theme branch December 5, 2023 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@p-offtermatt p-offtermatt p-offtermatt approved these changes

@MSalopek MSalopek MSalopek approved these changes

Assignees

@mmulji-ic mmulji-ic

Labels
C:ADR Assigned automatically by the PR labeler C:Docs Assigned automatically by the PR labeler S: ImprovingThings Improving things: Customer requests, performance improvements, reliability and usability scope: docs Improvements or additions to documentation
Projects
Cosmos Hub
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mmulji-ic @p-offtermatt @MSalopek