seekseek
/
ui
1
0
Fork 1
Code Issues 1 Pull Requests 1 Projects Releases Wiki Activity

Labels Milestones

WIP: supakeen/ui-improvements #1

Draft
supakeen wants to merge 10 commits from supakeen/ui:supakeen/ui-improvements into master
pull from: supakeen/ui:supakeen/ui-improvements
merge into: seekseek:master
Conversation 1 Commits 10 Files Changed 6
supakeen commented 2 years ago

CSS improvements

Layouting

For layouting a grid layout was chosen, it offers the same flexibility as before (and more, especially regarding media queries) and gets rid of superfluous wrapper elements.

Dark-Mode

CSS media queries are used for dark mode, the colors are very much a work-in-progress.

Limit Width

It is hard for people to scan wide lines on monitors. There are some limits set through media queries, these limits can be changed at will as the site needs more or less horizontal space.

Text

Minor updates to line-height.

Bugs

I can't get live-reload to work on the component CSS after I adjusted it to use CSS-variables, that means I can't really tell if I did that bit right. Could you take a look at that?

# CSS improvements ## Layouting For layouting a grid layout was chosen, it offers the same flexibility as before (and more, especially regarding media queries) and gets rid of superfluous wrapper elements. ## Dark-Mode CSS media queries are used for dark mode, the colors are very much a work-in-progress. ## Limit Width It is hard for people to scan wide lines on monitors. There are some limits set through media queries, these limits can be changed at will as the site needs more or less horizontal space. ## Text Minor updates to line-height. # Bugs I can't get live-reload to work on the component CSS after I adjusted it to use CSS-variables, that means I can't really tell if I did that bit right. Could you take a look at that?
supakeen added 8 commits 2 years ago
9b8425b094 Simplify layout and templates. 
By removing wrapper elements and switching to a CSS grid layout there
are overall less elements to style while providing the same
possibilities.
22023e63ee Add media queries for larger screens. 
It is hard for people to keep track of very wide pages (and
sentences), with these mediaqueries the width of the site is limited on
screens that are wider than a 1000, or 1400 pixels.
38925e7381 Dark mode through media query. 
Browsers can prefer a certain color scheme. This commit implements a new
dark color scheme for the website which is supposed to be a placeholder,
better colors could be chosen.
c8b92358bb Text line-height and link hovers, semantics. 
The general 'good readability' rule is to have a bit larger line-height,
aside from this I've added some interactivity to links by hovering over
them.

I also replaced `staticContent` with `article` for semantics.
6bdf36c5dd Cleanup of CSS. 
All units expressed in `rem`, invert logo in prefers-dark, padding
around the footer.
9a60ac7081 Left-align content. 
Aligning the content to the left allows for a more consistent experience
while resizing.
afd4b27437 Propagate CSS variables. 
CSS variables are used for the prefers-dark, propagate them to component
CSS as well.
supakeen added 1 commit 2 years ago
15666aa909 Bad editor. Bad. 
Fix mixed indentation and removal of tabs.
joepie91 requested changes 2 years ago
joepie91 left a comment
Owner

Thanks for the PR! I've left some comments on things to fix/discuss/etc. - ended up accidentally attaching them to public/css/style.css instead of src/css/style.css, but you get the idea :)

Thanks for the PR! I've left some comments on things to fix/discuss/etc. - ended up accidentally attaching them to `public/css/style.css` instead of `src/css/style.css`, but you get the idea :)
public/css/style.css Outdated
@ -22,4 +49,1 @@
.contents {
margin-bottom: 2em;
padding: .7em 0;
joepie91 commented 2 years ago
Owner

Removing this padding has caused the datasheet counter to feel 'cramped' against the dividing line of the header, and it should probably be reintroduced in some way.

Removing this padding has caused the datasheet counter to feel 'cramped' against the dividing line of the header, and it should probably be reintroduced in some way.
supakeen marked this conversation as resolved
public/css/style.css Outdated
@ -17,4 +40,0 @@
margin-left: 0.5em;
margin-right: 0.5em;
padding-bottom: .7em;
border-bottom: 2px solid black;
joepie91 commented 2 years ago
Owner

Can you restore the border thickness here? Matter of aesthetic preference and 'branding consistency' with the logo :)

Can you restore the border thickness here? Matter of aesthetic preference and 'branding consistency' with the logo :)
supakeen marked this conversation as resolved
public/css/style.css Outdated
@ -1,0 +9,4 @@
--invert: 0;
}
@media (prefers-color-scheme: dark) {
joepie91 commented 2 years ago
Owner

There are still a number of remaining issues with the dark mode theme, currently:

  • The 'datasheets' and 'beta' text in the logo is too low-contrast
  • The hyperlinks on the page are too low-contrast
  • The 'sources' list is too low-contrast
  • The manufacturer name and description text for datasheet search results are still dark gray, and therefore unreadable
  • The background feels a bit too black overall - something like #3b3b3b would probably be easier on the eyes
  • The dividing line between header/footer and content should probably be a bit more subtly colored
There are still a number of remaining issues with the dark mode theme, currently: - The 'datasheets' and 'beta' text in the logo is too low-contrast - The hyperlinks on the page are too low-contrast - The 'sources' list is too low-contrast - The manufacturer name and description text for datasheet search results are still dark gray, and therefore unreadable - The background feels a bit *too* black overall - something like #3b3b3b would probably be easier on the eyes - The dividing line between header/footer and content should probably be a bit more subtly colored
supakeen commented 2 years ago
Poster

Agreed with all points, but, for this part:

The manufacturer name and description text for datasheet search results are still dark gray, and therefore unreadable

That is the part I was talking about off-site, it seems the component CSS which does have the CSS-variables in it is not using them. If they do work for you, can you verify that it's the actual color that is wrong or the component CSS not being applied properly?

Agreed with all points, but, for this part: > The manufacturer name and description text for datasheet search results are still dark gray, and therefore unreadable That is the part I was talking about off-site, it seems the component CSS which does have the CSS-variables in it is not using them. If they do work for you, can you verify that it's the actual color that is wrong or the component CSS not being applied properly?
joepie91 commented 2 years ago
Owner

That is the part I was talking about off-site, it seems the component CSS which does have the CSS-variables in it is not using them.

Oops, caused by an outdated build tool having a bug. Fixed in c21a9fea09.

> That is the part I was talking about off-site, it seems the component CSS which does have the CSS-variables in it is not using them. Oops, caused by an outdated build tool having a bug. Fixed in c21a9fea09260650f5b38c93ed8722164ab3d9d9.
supakeen marked this conversation as resolved
public/css/style.css Outdated
@ -19,2 +40,2 @@
padding-bottom: .7em;
border-bottom: 2px solid black;
body {
padding: .5rem;
joepie91 commented 2 years ago
Owner

This padding is reduced too much on large screens; while reducing it makes sense for smaller screens, ideally it'd match the original padding for larger ones. Relatedly, the logo is very weirdly positioned now, with a much larger top margin than left margin.

This padding is reduced too much on large screens; while reducing it makes sense for smaller screens, ideally it'd match the original padding for larger ones. Relatedly, the logo is very weirdly positioned now, with a much larger top margin than left margin.
supakeen marked this conversation as resolved
public/css/style.css Outdated
@ -21,0 +40,4 @@
body {
padding: .5rem;
height: 100%;
display: grid;
joepie91 commented 2 years ago
Owner

While using a grid does solve some sizing issues on small screens in particular, it unfortunately also relegates the footer to the bottom of the page, rather than being permanently visible. Ideally the footer would always remain visible, as it's the place where visitors are invited to get involved with the project, which is a very important part of SeekSeek - it shouldn't just be something people use, but something that people build on collaboratively. Basically, the footer is the 'point of community contact' with visitors.

I'm not sure what the best way to accomplish this would be. The 'obvious' solution is to have a separate grid that keeps the footer in view and has the header+content in a scrollable box above it, but I'm not sure that eg. screenreaders would understand that the scrollable box is the main content of the page (like was obvious when the footer was just overlaid on the main page content).

While using a grid *does* solve some sizing issues on small screens in particular, it unfortunately also relegates the footer to the bottom of the page, rather than being permanently visible. Ideally the footer would always remain visible, as it's the place where visitors are invited to get involved with the project, which is a very important part of SeekSeek - it shouldn't just be something people *use*, but something that people build on collaboratively. Basically, the footer is the 'point of community contact' with visitors. I'm not sure what the best way to accomplish this would be. The 'obvious' solution is to have a separate grid that keeps the footer in view and has the header+content in a scrollable box above it, but I'm not sure that eg. screenreaders would understand that the scrollable box is the main content of the page (like was obvious when the footer was just overlaid on the main page content).
supakeen marked this conversation as resolved
src/css/style.css Outdated
@ -92,0 +127,4 @@
}
}
@media only screen and (min-width: 1000px) {
joepie91 commented 2 years ago
Owner

There's a strange 'jump' here when resizing - wouldn't it be better to just have a single maximum width, rather than deliberately shrinking the results below a certain screen size? Each individual search frontend will need its own CSS anyway to optimally display on all screen sizes, so I don't really see any value in 'leaving space' in this case.

There's a strange 'jump' here when resizing - wouldn't it be better to just have a single maximum width, rather than deliberately shrinking the results below a certain screen size? Each individual search frontend will need its own CSS anyway to optimally display on all screen sizes, so I don't really see any value in 'leaving space' in this case.
supakeen marked this conversation as resolved
src/frontend/components/datasheet-search.css Outdated
@ -4,2 +2,2 @@
color: black;
border: 1px solid teal;
font-size: 1.3rem;
background-color: var(--color-background-autention);
joepie91 commented 2 years ago
Owner

That should probably be attention, not autention :)

That should probably be `attention`, not `autention` :)
supakeen marked this conversation as resolved
src/views/_layout.jsx Outdated
@ -23,3 +16,1 @@
</div>
</div>
<div className="contents">
<header>
joepie91 commented 2 years ago
Owner

I'm not sure about removing the wrapper here - having the header line continue across the entire screen was a deliberate choice, in preparation of other search frontends being developed in the future with different usage of screen space (where it may well fill the entire screen).

While this does look at bit aesthetically neater right now, it would cause an inconsistent 'core UI' across different search engines in the future, as different search frontends would have a different 'header width'.

I'm not sure about removing the `wrapper` here - having the header line continue across the entire screen was a deliberate choice, in preparation of *other* search frontends being developed in the future with different usage of screen space (where it may well fill the entire screen). While this *does* look at bit aesthetically neater right now, it would cause an inconsistent 'core UI' across different search engines in the future, as different search frontends would have a different 'header width'.
supakeen force-pushed supakeen/ui-improvements from 15666aa909 to 13002057f8 2 years ago
supakeen commented 2 years ago
Poster

I've attended to most points mentioned, there are two things left to speak about (I think).

A better colorscheme for dark mode, the width of the header/site. I understand the need for consistency in the future but as long as there is a single frontend it might be prudent to pick the more pleasing look for now, and instead extend/rethink it when multiple frontends are available? :)

I've attended to most points mentioned, there are two things left to speak about (I think). A better colorscheme for dark mode, the width of the header/site. I understand the need for consistency in the future but as long as there is a single frontend it might be prudent to pick the more pleasing look for now, and instead extend/rethink it when multiple frontends are available? :)

Reviewers

joepie91 requested changes 2 years ago
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b supakeen-supakeen/ui-improvements master
git pull supakeen/ui-improvements

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff supakeen-supakeen/ui-improvements
git push origin master
Sign in to join this conversation.
Reviewers
Request review
No reviewers
joepie91
Labels
Apply labels
Clear labels
No items
No Label
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Set Project
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: seekseek/ui#1
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'supakeen/ui:supakeen/ui-improvements'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes