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

CSS initial special value #3566

Merged
merged 21 commits into from
Oct 23, 2023
Merged

CSS initial special value #3566

merged 21 commits into from
Oct 23, 2023

Conversation

willmcgugan
Copy link
Collaborator

@willmcgugan willmcgugan commented Oct 21, 2023 •
edited
Loading

  • Makes empty CSS rules an error
  • Adds initial to all rules, which restores default
  • Fixed issues with CSS error reporting duplicate errors
  • Set the max height of the DataTable to 100vh by default. This is because the previous value of 100% broke with horizontal layouts.
  • Added log message for startup time
  • Added xfail for CSS inheritance. Need to come back to this one.

@willmcgugan willmcgugan marked this pull request as draft October 21, 2023 11:39
@willmcgugan willmcgugan changed the title CSS unset CSS initial special value Oct 21, 2023
willmcgugan
willmcgugan commented Oct 22, 2023
View reviewed changes
src/textual/app.py Show resolved Hide resolved
src/textual/css/stylesheet.py Show resolved Hide resolved
@willmcgugan willmcgugan marked this pull request as ready for review October 22, 2023 11:08
davep
davep approved these changes Oct 23, 2023
View reviewed changes
src/textual/_layout.py Outdated Show resolved Hide resolved
@rodrigogiraoserrao rodrigogiraoserrao mentioned this pull request Oct 23, 2023
Closed
rodrigogiraoserrao
rodrigogiraoserrao reviewed Oct 23, 2023
View reviewed changes
Copy link
Contributor

@rodrigogiraoserrao rodrigogiraoserrao left a comment

Choose a reason for hiding this comment

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

Looks good to me.
I really wish you accept my recommendation of rewording a couple of comments where the English word “initial” and the new special value initial make reading the code hard.
But I didn't find anything worth blocking the PR for.

docs/guide/CSS.md Show resolved Hide resolved
src/textual/css/stylesheet.py Outdated Show resolved Hide resolved
src/textual/css/stylesheet.py Outdated Show resolved Hide resolved
src/textual/css/stylesheet.py Show resolved Hide resolved
src/textual/css/stylesheet.py Outdated Show resolved Hide resolved
src/textual/message_pump.py Outdated Show resolved Hide resolved
src/textual/css/stylesheet.py Show resolved Hide resolved
willmcgugan and others added 4 commits October 23, 2023 12:21
@willmcgugan @rodrigogiraoserrao
Update src/textual/css/stylesheet.py
0949ebd 
Co-authored-by: Rodrigo Girão Serrão <5621605+rodrigogiraoserrao@users.noreply.github.com>
@willmcgugan @rodrigogiraoserrao
Update src/textual/css/stylesheet.py
45d1a8e 
Co-authored-by: Rodrigo Girão Serrão <5621605+rodrigogiraoserrao@users.noreply.github.com>
@willmcgugan
restore arrange
16569d4
@willmcgugan @rodrigogiraoserrao
Apply suggestions from code review
84742c6 
Co-authored-by: Rodrigo Girão Serrão <5621605+rodrigogiraoserrao@users.noreply.github.com>
@willmcgugan willmcgugan merged commit 859cff3 into main Oct 23, 2023
23 checks passed
@willmcgugan willmcgugan deleted the css-unsert branch October 23, 2023 12:38
@rodrigogiraoserrao rodrigogiraoserrao mentioned this pull request Oct 24, 2023
Closed
rodrigogiraoserrao added a commit that referenced this pull request Oct 25, 2023
@rodrigogiraoserrao
Fix notifications CSS.
7b70b9a 
This was merged more or less at the same time as 'initial' was added to CSS but the tests weren't ran with that change.
Related PRs: #3566, #3531.
rodrigogiraoserrao added a commit that referenced this pull request Oct 25, 2023
@rodrigogiraoserrao
Fix notifications CSS.
87274cb 
This was merged more or less at the same time as 'initial' was added to CSS but the tests weren't ran with that change.
Related PRs: #3566, #3531.
@rodrigogiraoserrao rodrigogiraoserrao mentioned this pull request Oct 25, 2023
Merged
@TomJGooding TomJGooding mentioned this pull request Mar 12, 2024
Closed
TomJGooding added a commit to TomJGooding/textual that referenced this pull request Mar 14, 2024
@TomJGooding
fix(datatable): change max-height back to 100%
90895f2 
The `max-height` of the `DataTable` was changed to 100vh rather than
100% in Textualize#3566, because at the time this caused issues with auto height
containers, as described in Textualize#2975.

However this issue was later fixed in Textualize#3814. This PR changes the
`max-height` of the `DataTable` back to 100%, as 100vh will actually
break common layouts and seems no longer required to workaround this
auto height issue.

Fixes Textualize#4286.
@TomJGooding TomJGooding mentioned this pull request Mar 14, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rodrigogiraoserrao rodrigogiraoserrao rodrigogiraoserrao left review comments

@davep davep davep approved these changes

@darrenburns darrenburns Awaiting requested review from darrenburns

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

Successfully merging this pull request may close these issues.
3 participants
@willmcgugan @davep @rodrigogiraoserrao